-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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.
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
Add kubebuilder image build #28803
Add kubebuilder image build #28803
Conversation
Hi @yashsingh74. 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 Once the patch is verified, the new status will be reflected by the 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yashsingh74 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
testgrid-dashboards: sig-api-machinery-kubebuilder | ||
decorate: true | ||
branches: | ||
- ^master$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need to use master?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh it is not required.
Also, do we need to check against the Tag like mentioned here -https://github.com/kubernetes/test-infra/blob/master/config/jobs/image-pushing/k8s-staging-ingress-nginx.yaml#L10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it should be just against the tag. I don't think any changes in the branch need to trigger. @camilamacedo86 could you confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not trigger change anything on the master to build the proxy image. Only its own branch.
containers: | ||
- image: gcr.io/k8s-staging-test-infra/image-builder:v20230111-cd1b3caf9c | ||
command: | ||
- /run.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should build the proxy image right?
To build today we call: https://github.com/kubernetes-sigs/kubebuilder/blob/kube-rbac-proxy-releases/build/cloudbuild_kube-rbac-proxy.yaml
Which will call https://github.com/kubernetes-sigs/kubebuilder/blob/kube-rbac-proxy-releases/build/build.sh passing the tag version to do the job.
So, what this run.sh does?
What is the expected outcome when this PR get merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the run.sh will run initially which is present inside the image-builder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To build today we call: https://github.com/kubernetes-sigs/kubebuilder/blob/kube-rbac-proxy-releases/build/cloudbuild_kube-rbac-proxy.yaml
To be non-disruptive, do we need to create another cloudbuild.yaml as here? Not entirely sure so keeping the PR as a draft (kubernetes-sigs/kubebuilder#3246). cc: @camilamacedo86
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @varshaprasad96,
To be non-disruptive, do we need to create another cloudbuild.yaml as here? Not entirely sure so keeping the PR as a draft (kubernetes-sigs/kubebuilder#3246). cc: @camilamacedo86
I do not think that we need to have other cloudbuild.yaml
See that it is only a config file like gorelease one. We can call the same.
However, the new infra should build the images to the new registry
See that we inform the image registry in the build.sh: https://github.com/kubernetes-sigs/kubebuilder/blob/kube-rbac-proxy-releases/build/build.sh#L25
So, we need to change that to receive this info as an ARG OR just create another for the new infra.
containers: | ||
- image: gcr.io/k8s-staging-test-infra/image-builder:v20230111-cd1b3caf9c | ||
command: | ||
- /run.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To build today we call: https://github.com/kubernetes-sigs/kubebuilder/blob/kube-rbac-proxy-releases/build/cloudbuild_kube-rbac-proxy.yaml
To be non-disruptive, do we need to create another cloudbuild.yaml as here? Not entirely sure so keeping the PR as a draft (kubernetes-sigs/kubebuilder#3246). cc: @camilamacedo86
- --scratch-bucket=gs://k8s-staging-kubebuilder-gcb | ||
- --env-passthrough=PULL_BASE_REF | ||
- --with-git-dir | ||
- . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need the build-dir?
- name: post-kubebuilder-push-images | ||
cluster: k8s-infra-prow-build-trusted | ||
annotations: | ||
testgrid-dashboards: sig-api-machinery-kubebuilder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yashsingh74 from where did you find this value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ok-to-test |
PR needs rebase. 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. |
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
@yashsingh74: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
@k8s-triage-robot: Closed this PR. In response to this:
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-sigs/prow repository. |
Added the kubebuilder image build job.
Issue Ref: kubernetes-sigs/kubebuilder#3230