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.

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

build images using cloudbuild #123

Merged
merged 1 commit into from May 15, 2024

Conversation

upodroid
Copy link
Member

Part of #113
Part of kubernetes/k8s.io#6740

Right now, our postsubmit job runs in a pod which breaks our trusted cluster build policies.

I tested this build at https://console.cloud.google.com/cloud-build/builds;region=global/ce637e5c-b605-490f-b104-589b7dca23c6?project=k8s-infra-ii-sandbox

@BenTheElder @ameukam

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 21, 2024
Copy link

netlify bot commented Apr 21, 2024

Deploy Preview for k8s-prow ready!

Name Link
🔨 Latest commit d3a7f94
🔍 Latest deploy log https://app.netlify.com/sites/k8s-prow/deploys/66439a75ef1c2e0009b230e5
😎 Deploy Preview https://deploy-preview-123--k8s-prow.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

.gcloudignore Outdated
Copy link
Member Author

@upodroid upodroid Apr 21, 2024

Choose a reason for hiding this comment

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

an empty .gcloudignore allows GCB to upload the .git folder which is required by hack/prowimagebuilder to run git commands to fetch git status.

@ameukam
Copy link
Member

ameukam commented Apr 22, 2024

To my understanding, #113 states that we should attempt to promote prow components to registry.k8s.io ?

@upodroid
Copy link
Member Author

#113 requires Prow maintainers to start versioning prow and promote the release images to registry.k8s.io. In the meanwhile, we need to be able to publish and consume unversioned prow images from an AR registry called us-docker.pkg.dev/k8s-infra-prow/images

@ameukam
Copy link
Member

ameukam commented Apr 22, 2024

The location of the prow components is not a hard requirement to bootstrap a cluster for prow and we specially don't need to co-locate a AR registry and a GKE cluster.
Also a regional AR would be more sustainable and cost-effective.

@upodroid
Copy link
Member Author

upodroid commented Apr 22, 2024

Also a regional AR would be more sustainable and cost-effective.

Pricing is the same for both storage and bandwidth. https://cloud.google.com/artifact-registry/pricing#storage

prow and we specially don't need to co-locate a AR registry and a GKE cluster.

I'm doing this for convenience. Prow images are a special case and will need to be stored for more than 60 days(how long is TBD) and AR has a good security boundary, unlike GCR which interacts with GCS so a dedicated staging project makes no sense. The approach in k8s-prow where the cluster and images are in the project works for us.

@dims
Copy link
Member

dims commented May 4, 2024

/approve
/lgtm
/hold please remove hold as needed

@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, 2024
@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, 2024
@BenTheElder
Copy link
Member

Pricing is the same for both storage and bandwidth. https://cloud.google.com/artifact-registry/pricing#storage

currently

Prow images are a special case and will need to be stored for more than 60 days

They shouldn't be. Which other projects would we special-case?

The only special case I know currently is registry.k8s.io because we would have a circular dependency problem.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 14, 2024
@upodroid
Copy link
Member Author

/cc @droslean @matthyx

I would like to get this merged soon, thanks

@matthyx
Copy link
Contributor

matthyx commented May 14, 2024

/approve
/lgtm
Please remove the hold if @BenTheElder comment is addressed.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 14, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, matthyx, upodroid

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 14, 2024
@upodroid
Copy link
Member Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 15, 2024
@k8s-ci-robot k8s-ci-robot merged commit dd5d0ee into kubernetes-sigs:main May 15, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants