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

adds k8s-staging-csi-secrets-store image pushing prow config #17335

Merged
merged 1 commit into from
May 24, 2020
Merged

adds k8s-staging-csi-secrets-store image pushing prow config #17335

merged 1 commit into from
May 24, 2020

Conversation

claudiubelu
Copy link
Contributor

Adds the prow config needed for pushing the CSI secrets store driver to a staging registry, from where the images will be pushed.

The secrets-store-csi-driver project group has already been created: kubernetes/k8s.io#658

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 22, 2020
@claudiubelu claudiubelu mentioned this pull request Apr 22, 2020
Merged
@claudiubelu
Copy link
Contributor Author

/cc @aramase

To which testgrid-dashboard should this job report to?

@claudiubelu
Copy link
Contributor Author

Currently, the image name will be: gcr.io/k8s-staging-csi-secrets-store/secrets-store-csi, which is a bit redundant, IMO. I'm wondering if it's not better to have it as: gcr.io/k8s-staging-csi/secrets-store-csi, but that would mean sharing the gcr.io/k8s-staging-csi registry with other csi projects. I'm wondering if we can do that.

@aramase
Copy link
Member

aramase commented Apr 22, 2020

@claudiubelu Thank you for this. The dashboard would be - sig-auth-secrets-store-csi-driver

@aramase
Copy link
Member

aramase commented Apr 22, 2020

Currently, the image name will be: gcr.io/k8s-staging-csi-secrets-store/secrets-store-csi, which is a bit redundant, IMO. I'm wondering if it's not better to have it as: gcr.io/k8s-staging-csi/secrets-store-csi, but that would mean sharing the gcr.io/k8s-staging-csi registry with other csi projects. I'm wondering if we can do that.

The discussion in the PR was to have a common bucket for all csi images, but I don't think that's something that'll happen soon. We can change the image name to driver gcr.io/k8s-staging-csi-secrets-store/driver instead of having the redundant image name gcr.io/k8s-staging-csi-secrets-store/secrets-store-csi. @ritazh wdyt?

@ritazh
Copy link
Member

ritazh commented Apr 22, 2020

We can change the image name to driver gcr.io/k8s-staging-csi-secrets-store/driver instead of having the redundant image name gcr.io/k8s-staging-csi-secrets-store/secrets-store-csi

Sounds good!

@claudiubelu claudiubelu force-pushed the k8s-staging-csi-secret-store-image-job branch from e79fa65 to b82ce92 Compare April 22, 2020 18:46
@claudiubelu claudiubelu changed the title WIP: adds k8s-staging-csi-secrets-store image pushing prow config adds k8s-staging-csi-secrets-store image pushing prow config Apr 22, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 22, 2020
aramase
aramase approved these changes May 4, 2020
View reviewed changes
Copy link
Member

@aramase aramase left a comment

Choose a reason for hiding this comment

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

/lgtm

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

aramase commented May 4, 2020

/hold

(checking with @claudiubelu if the cluster needs to be k8s-infra-prow-build-trusted)

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 4, 2020
@claudiubelu claudiubelu mentioned this pull request May 11, 2020
Merged
@claudiubelu claudiubelu force-pushed the k8s-staging-csi-secret-store-image-job branch from b82ce92 to 51f55f8 Compare May 11, 2020 13:40
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 11, 2020
@claudiubelu claudiubelu force-pushed the k8s-staging-csi-secret-store-image-job branch 2 times, most recently from ed8e4fd to 5bd1000 Compare May 20, 2020 11:33
aramase
aramase reviewed May 20, 2020
View reviewed changes
Copy link
Member

@aramase aramase left a comment

Choose a reason for hiding this comment

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

/lgtm

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

aramase commented May 20, 2020

/assign @cblecker

cblecker
cblecker requested changes May 20, 2020
View reviewed changes
config/jobs/image-pushing/k8s-staging-csi-secrets-store.yaml Show resolved Hide resolved
config/jobs/image-pushing/k8s-staging-csi-secrets-store.yaml Outdated
Comment on lines 35 to 37
env:
- name: LOG_TO_STDOUT
value: "y"
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it.

config/jobs/image-pushing/k8s-staging-csi-secrets-store.yaml
Comment on lines +13 to +14
# we only need to run if necessary (e.g.: the version was bumped)
run_if_changed: '^docker\/'
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't match any file in your repo

Copy link
Member

Choose a reason for hiding this comment

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

There is a PR open for moving the files to different dir - kubernetes-sigs/secrets-store-csi-driver#189

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2020
@claudiubelu
adds k8s-staging-csi-secrets-store image pushing prow config
7778a67 
Adds the prow config needed for pushing the CSI secrets store driver to
a staging registry, from where the images will be pushed.
@claudiubelu claudiubelu force-pushed the k8s-staging-csi-secret-store-image-job branch from 5bd1000 to 7778a67 Compare May 21, 2020 13:32
@claudiubelu claudiubelu mentioned this pull request May 21, 2020
Closed
@cblecker
Copy link
Member

/lgtm
/approve
/hold cancel

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels May 24, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aramase, cblecker, claudiubelu

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 May 24, 2020
@k8s-ci-robot k8s-ci-robot merged commit 817c35c into kubernetes:master May 24, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.19 milestone May 24, 2020
@k8s-ci-robot
Copy link
Contributor

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

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

In response to this:

Adds the prow config needed for pushing the CSI secrets store driver to a staging registry, from where the images will be pushed.

The secrets-store-csi-driver project group has already been created: kubernetes/k8s.io#658

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aramase aramase aramase approved these changes

@cblecker cblecker cblecker requested changes

@cjwagner cjwagner Awaiting requested review from cjwagner

@Katharine Katharine Awaiting requested review from Katharine

Assignees

@cblecker cblecker

@aramase aramase

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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.19
Development

Successfully merging this pull request may close these issues.
5 participants
@claudiubelu @aramase @ritazh @cblecker @k8s-ci-robot