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

Add helm chart repository #2377

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

IrvingMg
Copy link
Contributor

@IrvingMg IrvingMg commented Jun 7, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add Helm chart repository.

Which issue(s) this PR fixes:

Fixes #2311

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Experimental support for helm charts in the gcr.io/k8s-staging-kueue/charts/kueue repository

@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 Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Jun 7, 2024
@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 Jun 7, 2024
Copy link

netlify bot commented Jun 7, 2024

Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit 789677c
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/666c5ceb7568cc00082a027a

Copy link
Contributor

@trasc trasc left a comment

Choose a reason for hiding this comment

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

nits

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 11, 2024
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 13, 2024
@IrvingMg IrvingMg force-pushed the feature/helm-chart-repo branch 3 times, most recently from 10a2a76 to 84b783e Compare June 13, 2024 15:01
hack/push-chart.sh Outdated Show resolved Hide resolved
hack/push-chart.sh Outdated Show resolved Hide resolved
@trasc
Copy link
Contributor

trasc commented Jun 14, 2024

/assign

hack/push-chart.sh Outdated Show resolved Hide resolved
@IrvingMg IrvingMg marked this pull request as ready for review June 14, 2024 14:09
@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 Jun 14, 2024
@k8s-ci-robot k8s-ci-robot requested a review from trasc June 14, 2024 14:09
Copy link
Contributor

@trasc trasc left a comment

Choose a reason for hiding this comment

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

After this is merged and we have "production confirmation" that it works as we expect we showed follow-up on:

  • check if we are also able to promote the charts to registry.k8s.io
  • drop the old build method (release artifacts)
  • update the install related documentation

/lgtm

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

LGTM label has been added.

Git tree hash: dceb1b97d8c764987b722746d0f1e7702bcc683e

@trasc
Copy link
Contributor

trasc commented Jun 14, 2024

/assign @alculquicondor

@mruoss
Copy link

mruoss commented Jun 15, 2024

After this is merged and we have "production confirmation" that it works as we expect we showed follow-up on:

  • check if we are also able to promote the charts to registry.k8s.io
  • drop the old build method (release artifacts)
  • update the install related documentation

/lgtm

A package publication on https://artifacthub.io/ whould be nice as a follow up, too.

@alculquicondor
Copy link
Contributor

/release-note-edit

Publish Helm charts in registry.k8s.io/kueue/charts repository

Assuming that we can promote the image there. If it doesn't work, we can revert this PR.

@alculquicondor
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, IrvingMg

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 Jun 17, 2024
@k8s-ci-robot k8s-ci-robot merged commit d67883e into kubernetes-sigs:main Jun 17, 2024
16 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.8 milestone Jun 17, 2024
@trasc trasc deleted the feature/helm-chart-repo branch June 18, 2024 05:25
@alculquicondor
Copy link
Contributor

It looks like the objects are properly listed in gcr.io/k8s-staging-kueue/charts/kueue

Should it be gcr.io/k8s-staging-kueue/kueue/charts instead?

@alculquicondor
Copy link
Contributor

Also, can you test those objects to see if they work properly?

PBundyra pushed a commit to PBundyra/kueue that referenced this pull request Jun 19, 2024
@IrvingMg
Copy link
Contributor Author

  • It looks like the objects are properly listed in gcr.io/k8s-staging-kueue/charts/kueue

    Should it be gcr.io/k8s-staging-kueue/kueue/charts instead?

It's possible to change the path but Helm creates a subdirectory for the chart at the end. So if we change it, the resulting path would be gcr.io/k8s-staging-kueue/kueue/charts/kueue.

  • Also, can you test those objects to see if they work properly?

It seems to be working as expected, I ran:

helm install kueue oci://gcr.io/k8s-staging-kueue/charts/kueue --version v20240619-v0.8.0-devel-54-g81ad017a

and got

Pulled: gcr.io/k8s-staging-kueue/charts/kueue:v20240619-v0.8.0-devel-54-g81ad017a
Digest: sha256:7f628d9e268fb091d075aa7bd043254219ade42845c32f75184e4c28320326c7
NAME: kueue
LAST DEPLOYED: Wed Jun 19 11:38:04 2024
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None

@alculquicondor
Copy link
Contributor

I left a question in the k8s-infra slack channel https://kubernetes.slack.com/archives/CCK68P2Q2/p1718819420490889

@BenTheElder
Copy link
Member

BenTheElder commented Jun 21, 2024

Nobody should be consuming from gcr.io/k8s-staging-kueue directly anyhow (in fact we will have to move the staging locations to artifact registry later this year) so the path doesn't matter all that much there, when promoted to registry.k8s.io the path can be different.

@alculquicondor
Copy link
Contributor

From slack thread:

Please start publishing staging images for kueue to us-central1-docker.pkg.dev/k8s-staging-images/kueue

Fiona-Waters pushed a commit to Fiona-Waters/kueue that referenced this pull request Jun 25, 2024
@alculquicondor
Copy link
Contributor

/release-note-edit

Experimental support for helm charts in the gcr.io/k8s-staging-kueue/charts/kueue repository

Because #2476 didn't merge and I don't know if it's safe to do so.

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.

Helm chart repository
6 participants