Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Image Push Job For Kubernetes CSI #16942

Merged
merged 2 commits into from
Apr 1, 2020
Merged

Add Image Push Job For Kubernetes CSI #16942

merged 2 commits into from
Apr 1, 2020

Conversation

vitt-bagal
Copy link
Contributor

@vitt-bagal vitt-bagal commented Mar 25, 2020 •
edited
Loading

Adding image push job for the Kubernetes CSI packages for automating Image build, push(to staging area) process .

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 25, 2020
@k8s-ci-robot
Copy link
Contributor

Welcome @vitt-bagal!

It looks like this is your first PR to kubernetes/test-infra 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/test-infra has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot
Copy link
Contributor

Hi @vitt-bagal. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 25, 2020
@vitt-bagal
Copy link
Contributor Author

@pohly Could you please have a look ?
References: kubernetes-csi/node-driver-registrar#67 and kubernetes-csi/node-driver-registrar#68

pohly
pohly requested changes Mar 27, 2020
View reviewed changes
Copy link
Contributor

@pohly pohly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor comment. Overall this looks like it might work, as far as I understand the process.

config/jobs/image-pushing/k8s-staging-csi.yaml
testgrid-dashboards: sig-storage-csi-node-driver-registrar
decorate: true
branches:
- ^master$
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment explaining that this is for publishing the "canary" images, something that we only do on the master branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment

config/jobs/image-pushing/k8s-staging-csi.yaml
@@ -0,0 +1,29 @@
postsubmits:
# This is the github repo we'll build from. This block needs to be repeated
# for each repo.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually we'll probably want to have a generator script for all the various repos. There are too many to of them to create and maintain this file manually. But for now as a test it's okay.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 27, 2020
@vitt-bagal vitt-bagal mentioned this pull request Mar 27, 2020
Merged
pohly
pohly approved these changes Mar 27, 2020
View reviewed changes
Copy link
Contributor

@pohly pohly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 27, 2020
@spiffxp
Copy link
Member

spiffxp commented Mar 27, 2020

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 27, 2020
@spiffxp
Copy link
Member

spiffxp commented Mar 27, 2020

/approve

@vitt-bagal
Copy link
Contributor Author

/assign @BenTheElder

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Katharine, pohly, spiffxp, vitt-bagal

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 1, 2020
@k8s-ci-robot k8s-ci-robot merged commit 8c28b7a into kubernetes:master Apr 1, 2020
@k8s-ci-robot
Copy link
Contributor

@vitt-bagal: Updated the job-config configmap in namespace default at cluster default using the following files:

  • key k8s-staging-csi.yaml using file config/jobs/image-pushing/k8s-staging-csi.yaml

In response to this:

Adding image push job for the Kubernetes CSI packages for automating Image build, push(to staging area) process .

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@vitt-bagal
Copy link
Contributor Author

@pohly @Katharine Looking at prow it seems that postsubmit job for nodedriver registrar is not executed. Do we need to wait for a commit to be merged to node driver registrar master ?

@BenTheElder
Copy link
Member

Yes. Postsubmits run post submit (after a merge), there's also a branch regex optionally that would restrict to only certain pushes.

@vitt-bagal
Copy link
Contributor Author

Thanks @BenTheElder

@pohly As Postsubmit job runs after a merge, Is it possible for us to commit something in node driver registrar master ? e.g. sync release tool ?

@pohly
Copy link
Contributor

pohly commented Apr 7, 2020

I created kubernetes-csi/node-driver-registrar#71

@vitt-bagal
Copy link
Contributor Author

@pohly @BenTheElder Image push job for node-driver-registrar got triggered after PR#71, however it fails: https://prow.k8s.io/view/gcs/kubernetes-jenkins/logs/post-node-driver-registrar-push-images/1247569283886616576 Could you please check once?

@BenTheElder
Copy link
Member

Hi, please check the "artifacts" link to see more logs. I unfortunately really don't have the bandwidth to debug everyone's CI.

@vitt-bagal
Copy link
Contributor Author

The build.log from artifacts point to a url 'https://console.cloud.google.com/cloud-build/builds/ac677e83-ad64-4ec3-8204-f0727b714682?project=874328413592' which gives us error You do not have sufficient permissions to view this page. @pohly Can you access the logs?

@BenTheElder
Copy link
Member

BenTheElder commented Apr 13, 2020 via email

@spiffxp
Copy link
Member

spiffxp commented Apr 13, 2020

@pohly @vitt-bagal you should probably open a PR against https://github.com/kubernetes/k8s.io/blob/master/groups/groups.yaml#L705-L712, I'm not aware of the history of how that group was created, but I doubt it should be just @dims. That would let you view the cloud-build link.

But I agree I'm confused why the logs weren't downloaded to artifacts/ in the first place. Maybe a flake.

Here's what I'm able to see

FETCHSOURCE
Fetching storage object: gs://k8s-staging-csi-gcb/source/1586278720.99-3d318855304f43258f3cef5b72318550.tgz#1586278721356113
Copying gs://k8s-staging-csi-gcb/source/1586278720.99-3d318855304f43258f3cef5b72318550.tgz#1586278721356113...
/ [0 files][    0.0 B/  1.7 MiB]                                                
/ [1 files][  1.7 MiB/  1.7 MiB]                                                
Operation completed over 1 objects/1.7 MiB.                                      
BUILD
Pulling image: gcr.io/k8s-testimages/gcb-docker-gcloud:v20190906-745fed4
v20190906-745fed4: Pulling from k8s-testimages/gcb-docker-gcloud
9d48c3bd43c5: Pulling fs layer
7f94eaf8af20: Pulling fs layer
9fe9984849c1: Pulling fs layer
cf0db633a67d: Pulling fs layer
0f7136d71739: Pulling fs layer
36b51914f992: Pulling fs layer
b5f72b4b9a52: Pulling fs layer
cf0db633a67d: Waiting
0f7136d71739: Waiting
36b51914f992: Waiting
b5f72b4b9a52: Waiting
9d48c3bd43c5: Verifying Checksum
9d48c3bd43c5: Download complete
9fe9984849c1: Verifying Checksum
9fe9984849c1: Download complete
7f94eaf8af20: Verifying Checksum
7f94eaf8af20: Download complete
0f7136d71739: Verifying Checksum
0f7136d71739: Download complete
36b51914f992: Verifying Checksum
36b51914f992: Download complete
9d48c3bd43c5: Pull complete
7f94eaf8af20: Pull complete
9fe9984849c1: Pull complete
cf0db633a67d: Verifying Checksum
cf0db633a67d: Download complete
b5f72b4b9a52: Verifying Checksum
b5f72b4b9a52: Download complete
cf0db633a67d: Pull complete
0f7136d71739: Pull complete
36b51914f992: Pull complete
b5f72b4b9a52: Pull complete
Digest: sha256:f17f1d2a58b8273e184a7c9f89a91d0193a6f458167891115868781610467ddd
Status: Downloaded newer image for gcr.io/k8s-testimages/gcb-docker-gcloud:v20190906-745fed4
gcr.io/k8s-testimages/gcb-docker-gcloud:v20190906-745fed4
unknown flag: --use
See 'docker --help'.

@dims
Copy link
Member

dims commented Apr 14, 2020

This was way before we had automation:
https://github.com/kubernetes/k8s.io/pull/248/files#diff-8b242825ad9027b6efead6a412418c51R72-R77

It was mostly created as a placeholder to try things and no one got added afterwards. +1 to update the list of folks

@pohly
Copy link
Contributor

pohly commented Apr 14, 2020

+1 to update the list of folks

So who should be on that list?

@vitt-bagal vitt-bagal mentioned this pull request Apr 15, 2020
Merged
@vitt-bagal
Copy link
Contributor Author

There were 2 builds triggered yesterday and for both, logs not downloaded by tooling again. Till the group membership is sorted, can @dims or @spiffxp help us with the logs please?

@BenTheElder
Copy link
Member

/uncc
I don't own these resources, leaving it to dims and spiffxp.

@k8s-ci-robot k8s-ci-robot removed the request for review from BenTheElder April 16, 2020 08:09
@dims
Copy link
Member

dims commented Apr 20, 2020

the build is failing as we have a older docker version, see full log : https://paste.centos.org/view/raw/f725c290

i have a PR in test-infra for bumping gcb-docker-gcloud image to newer golang and docker - #17299

As for the list of people for that ACL for the job, please add all the leads for sig-storage, technical leads and any one that takes care of the csi images

pohly added a commit to pohly/k8s.io that referenced this pull request Apr 20, 2020
@pohly
groups: add SIG-Storage members to k8s-infra-staging-csi
60e5c43 
Right now, none of the people involved with SIG-Storage have access to
logs when staging
fails (kubernetes/test-infra#16942 (comment)). As
suggested in that
discussion (kubernetes/test-infra#16942 (comment)),
I am adding the current SIG chairs and tech leads plus myself as
someone who has been working on image publishing.
@pohly pohly mentioned this pull request Apr 20, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pohly pohly pohly approved these changes

@Katharine Katharine Katharine approved these changes

@cjwagner cjwagner Awaiting requested review from cjwagner

Assignees

@pohly pohly

@Katharine Katharine

@BenTheElder BenTheElder

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@vitt-bagal @k8s-ci-robot @spiffxp @BenTheElder @pohly @dims @Katharine