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

Publish images to new staging registry #2476

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

IrvingMg
Copy link
Contributor

@IrvingMg IrvingMg commented Jun 25, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

As discussed in #2377 (comment) and https://kubernetes.slack.com/archives/CCK68P2Q2/p1718819420490889, we should start publishing Kueue images to us-central1-docker.pkg.dev/k8s-staging-images/kueue.

Moreover, once kubernetes/k8s.io#6910 is merged, we're going to have a staging repo for charts in us-central1-docker.pkg.dev/k8s-staging-images/charts.

Which issue(s) this PR fixes:

Relates to #2377

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Publish images via artifact registry

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@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. release-note-none Denotes a PR that doesn't merit a release note. kind/feature Categorizes issue or PR as related to a new feature. 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 Jun 25, 2024
Copy link

netlify bot commented Jun 25, 2024

Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit 5d66190
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/669f51a620ae6400080efeab

hack/push-chart.sh Outdated Show resolved Hide resolved
@IrvingMg
Copy link
Contributor Author

IrvingMg commented Jul 1, 2024

/hold

Waiting for kubernetes/k8s.io#6910 to be merged.

@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 Jul 1, 2024
@IrvingMg IrvingMg marked this pull request as ready for review July 1, 2024 09:58
@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 Jul 1, 2024
@alculquicondor
Copy link
Contributor

@upodroid would publishing images to us-central1-docker.pkg.dev/k8s-staging-images/kueue work now, or do we need any changes to https://github.com/kubernetes/k8s.io/blob/main/registry.k8s.io/manifests/k8s-staging-kueue/promoter-manifest.yaml as well?

@alculquicondor
Copy link
Contributor

@upodroid, could you provide some clarity around the state of the repository?

Copy link
Member

@upodroid upodroid left a comment

Choose a reason for hiding this comment

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

The repositories are ready but we need to amend the postsubmit to push use it.

We need to open 2 PRs in test-infra and k8s.io repo which I'll do for you.

@@ -49,7 +49,7 @@ The following table lists the configurable parameters of the kueue chart and the
| `enablePrometheus` | enable Prometheus | `false` |
| `enableCertManager` | enable CertManager | `false` |
| `controllerManager.kubeRbacProxy.image` | controllerManager.kubeRbacProxy's image | `gcr.io/kubebuilder/kube-rbac-proxy:v0.8.0` |
| `controllerManager.manager.image` | controllerManager.manager's image | `gcr.io/k8s-staging-kueue/kueue:main` |
| `controllerManager.manager.image` | controllerManager.manager's image | `us-central1-docker.pkg.dev/k8s-staging-images/kueue:main` |
Copy link
Member

Choose a reason for hiding this comment

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

Can we make sure that the staging registries never appear in an end-user facing doc/helm chart? This should be registry.k8s.io/kueue/kueue

Copy link
Contributor Author

@IrvingMg IrvingMg Jul 19, 2024

Choose a reason for hiding this comment

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

I think it's possible to have only once kueue in the image name, isn't? Or should we have kueue/kueue?

Copy link
Member

Choose a reason for hiding this comment

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

us-central1-docker.pkg.dev/k8s-staging-images/kueue is the name of the registry so a kueue image would be referenced as us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue:latest

Copy link
Contributor

Choose a reason for hiding this comment

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

If someone is trying to use the charts from source, they currently point to the staging repo, so that they can get the latest build.
Alternatively, they can use the released chart https://github.com/kubernetes-sigs/kueue/releases/tag/v0.8.0, which points to registry.k8s.io

Copy link
Contributor Author

@IrvingMg IrvingMg Jul 19, 2024

Choose a reason for hiding this comment

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

@upodroid , As I understand, having kueue/kueue creates a kueue subdirectory in the registry and I think we can have the images in the root using only us-central1-docker.pkg.dev/k8s-staging-images/kueue, WDYT?

Copy link
Contributor Author

@IrvingMg IrvingMg Jul 19, 2024

Choose a reason for hiding this comment

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

I think the parent directory wouldn't change. Keeping only one kueue in the name will still store the images in the kueue directory. It would look like this

Screenshot 2024-07-19 at 20 57 04

Adding an extra /kueue will just create a subdirectory inside of the existing kueue directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

uhmm, I see. Ok then.

Copy link
Member

Choose a reason for hiding this comment

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

That is expected. I just pushed a prod image to the kueue registry repo to demonstrate this.

https://console.cloud.google.com/artifacts/docker/k8s-staging-images/us-central1/kueue

 mahamed  MAHAALI-M-2PY9  ~  Desktop  Git  $  gcrane cp registry.k8s.io/kueue/kueue:v0.7.1 us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue:v0.7.1
2024/07/19 22:21:00 Copying from registry.k8s.io/kueue/kueue:v0.7.1 to us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue:v0.7.1
2024/07/19 22:21:07 pushed blob: sha256:e8d9a567199d7a318c875f2558a679ba8a924f817afacbb428afc3ffe6be6828
2024/07/19 22:21:07 pushed blob: sha256:b6824ed73363f94b3b2b44084c51c31bc32af77a96861d49e16f91e3ab6bed71
2024/07/19 22:21:07 pushed blob: sha256:f531499c6b730fc55a63e5ade55ce2c849bbf03f894248e3a2092689e3749312
2024/07/19 22:21:08 pushed blob: sha256:058cf3d8c2ba04ad7c064698c08c5e886a8623c0ad6171b8d72684253534417d
2024/07/19 22:21:10 pushed blob: sha256:7c12895b777bcaa8ccae0605b4de635b68fc32d60fa08f421dc3818bf55ee212
2024/07/19 22:21:11 pushed blob: sha256:5664b15f108bf9436ce3312090a767300800edbbfd4511aa1a6d64357024d5dd
2024/07/19 22:21:11 pushed blob: sha256:33e068de264953dfdc9f9ada207e76b61159721fd64a4820b320d05133a55fb8
2024/07/19 22:21:11 pushed blob: sha256:27be814a09ebd97fac6fb7b82d19f117185e90601009df3fbab6f442f85cd6b3
2024/07/19 22:21:14 pushed blob: sha256:4aa0ea1413d37a58615488592a0b827ea4b2e48fa5a77cf707d0e35f025e613f
2024/07/19 22:21:14 pushed blob: sha256:da7816fa955ea24533c388143c78804c28682eef99b4ee3723b548c70148bba6
2024/07/19 22:21:15 pushed blob: sha256:9aee425378d2c16cd44177dc54a274b312897f5860a8e78fdfda555a0d79dd71
2024/07/19 22:21:17 pushed blob: sha256:3bafb296168895177ec856d53d67f0fb7bf1aabeda625dcf441db11f91cc8eef
2024/07/19 22:21:18 pushed blob: sha256:acb81118b6a57035f85d451554eb802d0b1f8d55ea52298500993d7c7c94549a
2024/07/19 22:21:18 pushed blob: sha256:281a73ce7346f6fc4719c9831e40b62c544bdd9d0d789d292e261321f6b7c4a5
2024/07/19 22:21:19 pushed blob: sha256:ce670d28e15e760d7119c950c8cff8ffa8a66a12a1153bd523b4c7874296fd9f
2024/07/19 22:22:45 pushed blob: sha256:914d4d4dc529ce7809c22aff484e3cd65b5e86cd38f86c997e5b5357797a4ad8
2024/07/19 22:22:46 us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue@sha256:a2e0b9228d447f4cfbe0fd5d7cead3eae5f2e3c76c1a4a7bf26e378c4ec3c318: digest: sha256:a2e0b9228d447f4cfbe0fd5d7cead3eae5f2e3c76c1a4a7bf26e378c4ec3c318 size: 2647
2024/07/19 22:22:53 pushed blob: sha256:76056cc213d745ef36399d1ee00f602e395d7f7f8dd33932c106855bb23aaffd
2024/07/19 22:22:53 us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue@sha256:866f53b4c619d0a5f0dcccfce99ca63db72e4e83e9df51885559be68fde3b7da: digest: sha256:866f53b4c619d0a5f0dcccfce99ca63db72e4e83e9df51885559be68fde3b7da size: 2647
2024/07/19 22:22:54 pushed blob: sha256:02c4476ad2591340f072637bff6d0886dc936cb4033f124da10ed87775e62af7
2024/07/19 22:22:54 us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue@sha256:02876d2f01c8a4e6d43c264e1df21806a275769112039c54dc836c9d840f8927: digest: sha256:02876d2f01c8a4e6d43c264e1df21806a275769112039c54dc836c9d840f8927 size: 2647
2024/07/19 22:23:00 pushed blob: sha256:15d568dbb53179097ef15e2a7472ce21c492af57dfc04c91c760e04eb008bfa6
2024/07/19 22:23:01 us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue@sha256:d906dbb83f5c30d0687ecc55a04f6f3a82aaa8d56f2d17f27b0989f3644d75a2: digest: sha256:d906dbb83f5c30d0687ecc55a04f6f3a82aaa8d56f2d17f27b0989f3644d75a2 size: 2647
2024/07/19 22:23:01 us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue:v0.7.1: digest: sha256:98a2eb0e6fed19a33786b0da07332c6be2f52d07a3b2b085b369898b3b11b81e size: 1251

Copy link
Member

Choose a reason for hiding this comment

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

The other images of the kueue project will be at the same level as the kueue image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I've updated the registry name to us-central1-docker.pkg.dev/k8s-staging-images/kueue so that we can reference the image as us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue:latest.

@@ -21,7 +21,7 @@ controllerManager:
pullPolicy: IfNotPresent
manager:
image:
repository: gcr.io/k8s-staging-kueue/kueue
repository: us-central1-docker.pkg.dev/k8s-staging-images/kueue
Copy link
Member

Choose a reason for hiding this comment

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

Same as before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Added extra /kueue.

cloudbuild.yaml Outdated
@@ -12,7 +12,7 @@ steps:
- importer-image-push
- helm-chart-push
env:
- IMAGE_REGISTRY=gcr.io/$PROJECT_ID
- IMAGE_REGISTRY=us-central1-docker.pkg.dev/k8s-staging-images
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be us-central1-docker.pkg.dev/k8s-staging-images/kueue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if we need to add kueue to the image registry, as the current image name is $(IMAGE_REGISTRY)/kueue which matches the registry name already.

Copy link
Contributor

Choose a reason for hiding this comment

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

correct, this is the name of the registry, not the repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to us-central1-docker.pkg.dev/k8s-staging-images/kueue.

https://gcr.io/k8s-staging-kueue/importer, for example:
`gcr.io/k8s-staging-kueue/importer:main-latest`
https://us-central1-docker.pkg.dev/k8s-staging-images/kueue/importer, for example:
`us-central1-docker.pkg.dev/k8s-staging-images/kueue/importer:main-latest`
Copy link
Member

Choose a reason for hiding this comment

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

Promote this image to registry.k8s.io and ask users to use it from registry.k8s.io/kueue/importer:vX.Y.Z

Copy link
Contributor

Choose a reason for hiding this comment

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

We can do it in a follow up. Right now, promoting this image is not part of our release process.

@alculquicondor
Copy link
Contributor

@tenzen-y @mimowo I'll leave this up to you.

@IrvingMg IrvingMg force-pushed the update-image-registry branch 2 times, most recently from 6a74392 to 35e2c0a Compare July 22, 2024 10:33
.github/ISSUE_TEMPLATE/NEW_RELEASE.md Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Copy link
Member

@tenzen-y tenzen-y left a comment

Choose a reason for hiding this comment

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

Thank you!
/lgtm
/approve

/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 Jul 23, 2024
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 23, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: c2347f2282c44a65a35627a49e1c6f05da997aee

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: IrvingMg, tenzen-y, 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 Jul 23, 2024
@mimowo
Copy link
Contributor

mimowo commented Jul 23, 2024

/hold
Just so that kubernetes/test-infra#33034 merges first as asked in the other PR description "Please merge this PR just before you merge the kueue PR."

@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 Jul 23, 2024
@tenzen-y
Copy link
Member

/hold Just so that kubernetes/test-infra#33034 merges first as asked in the other PR description "Please merge this PR just before you merge the kueue PR."

Oops. Good catch. Thank you!

@tenzen-y
Copy link
Member

/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 Jul 23, 2024
@k8s-ci-robot k8s-ci-robot merged commit d6cbd8c into kubernetes-sigs:main Jul 23, 2024
16 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.9 milestone Jul 23, 2024
@IrvingMg IrvingMg deleted the update-image-registry branch July 23, 2024 11:27
@alculquicondor
Copy link
Contributor

/cherry-pick release-0.8

@k8s-infra-cherrypick-robot
Copy link
Contributor

@alculquicondor: #2476 failed to apply on top of branch "release-0.8":

Applying: Publish images to new staging registry
Using index info to reconstruct a base tree...
M	.github/ISSUE_TEMPLATE/NEW_RELEASE.md
M	Makefile
M	Makefile-test.mk
M	cloudbuild.yaml
M	config/components/manager/kustomization.yaml
Falling back to patching base and 3-way merge...
Auto-merging config/components/manager/kustomization.yaml
CONFLICT (content): Merge conflict in config/components/manager/kustomization.yaml
Auto-merging cloudbuild.yaml
Auto-merging Makefile-test.mk
Auto-merging Makefile
Auto-merging .github/ISSUE_TEMPLATE/NEW_RELEASE.md
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Publish images to new staging registry
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-0.8

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.

@alculquicondor
Copy link
Contributor

/release-note-edit

Images are published via artifact registry

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Aug 13, 2024
@alculquicondor
Copy link
Contributor

/release-note-edit

Publish images via artifact registry

k8s-ci-robot pushed a commit that referenced this pull request Aug 13, 2024
#2832)

* Publish images to new staging registry

* Update image registry name

---------

Co-authored-by: Irving Mondragón <[email protected]>
kannon92 pushed a commit to openshift-kannon92/kubernetes-sigs-kueue that referenced this pull request Nov 19, 2024
* Publish images to new staging registry

* Update image registry name
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. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants