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

feat(Chart): 📦 add optional separated chart for CRDs #1223

Merged
merged 46 commits into from
Jan 10, 2025
Merged

feat(Chart): 📦 add optional separated chart for CRDs #1223

merged 46 commits into from
Jan 10, 2025

Conversation

darkweaver87
Copy link
Contributor

@darkweaver87 darkweaver87 commented Oct 14, 2024 •
edited
Loading

What does this PR do?

This PR introduces a separate chart used by the main one to install CRDs.
The dependency between the main chart and the CRDs one will be added once CRDs one will be released.

Motivation

#1141
#1209

More

  • Yes, I updated the tests accordingly
  • Yes, I updated the schema accordingly
  • Yes, I ran make test and all the tests passed

@darkweaver87 darkweaver87 added kind/enhancement New feature or request kind/proposal a proposal that needs to be discussed. labels Oct 14, 2024
@darkweaver87 darkweaver87 force-pushed the feat/crds branch 2 times, most recently from bd35651 to b674cf7 Compare October 14, 2024 08:39
@darkweaver87 darkweaver87 changed the title feat: 📦 add separated chart for CRDs feat!: 📦 add separated chart for CRDs Oct 14, 2024
mloiseleur
mloiseleur reviewed Oct 14, 2024
View reviewed changes
traefik/values.yaml Outdated Show resolved Hide resolved
@darkweaver87 darkweaver87 force-pushed the feat/crds branch 9 times, most recently from 9087b4b to e1eb8a0 Compare October 14, 2024 14:53
@darkweaver87 darkweaver87 mentioned this pull request Oct 15, 2024
Merged
3 tasks
@mloiseleur mloiseleur linked an issue Oct 16, 2024 that may be closed by this pull request
CRD only Helm chart for Traefik #1141
Closed
2 tasks
@rgdev rgdev mentioned this pull request Oct 17, 2024
Open
mloiseleur
mloiseleur requested changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

  1. The CI needs to be tested, with a tag on your fork.
  2. The documented CRDs upgrade process should still work. I see (at least) two options:
    a. Keep and copy crd files
    b. Update CRD upgrade instruction to use them in the new directory

@darkweaver87 darkweaver87 force-pushed the feat/crds branch 2 times, most recently from e9619df to 6a67459 Compare October 21, 2024 06:50
@darkweaver87
Copy link
Contributor Author

darkweaver87 commented Oct 21, 2024 •
edited
Loading

Tested on my own account as seen with you 👍

@darkweaver87 darkweaver87 force-pushed the feat/crds branch 2 times, most recently from a53e8f1 to 59d7ecc Compare January 8, 2025 14:51
darkweaver87
darkweaver87 commented Jan 8, 2025
View reviewed changes
README.md Outdated Show resolved Hide resolved
mloiseleur
mloiseleur reviewed Jan 8, 2025
View reviewed changes
traefik-crds/values.yaml Outdated Show resolved Hide resolved
mloiseleur
mloiseleur reviewed Jan 10, 2025
View reviewed changes
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
darkweaver87 and others added 2 commits January 10, 2025 09:25
@darkweaver87 @mloiseleur
fix: apply suggestions from @mloiseleur's code review
de095f0 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@darkweaver87
fix: CI
b6bc446
mloiseleur
mloiseleur reviewed Jan 10, 2025
View reviewed changes
README.md Show resolved Hide resolved
mloiseleur
mloiseleur reviewed Jan 10, 2025
View reviewed changes
README.md Outdated Show resolved Hide resolved
darkweaver87 and others added 2 commits January 10, 2025 09:57
@darkweaver87 @mloiseleur
fix: apply suggestions from @mloiseleur's code review
fdbf4b6 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@darkweaver87
fix: CI
b99068e
mloiseleur
mloiseleur approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

LGTM

@mloiseleur mloiseleur added status/3-needs-merge and removed bot/no-merge kind/proposal a proposal that needs to be discussed. labels Jan 10, 2025
@traefiker traefiker merged commit 057783a into traefik:master Jan 10, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mloiseleur mloiseleur mloiseleur approved these changes

Assignees
No one assigned
Labels
kind/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Don’t bundle Gateway API CRDs CRD only Helm chart for Traefik
5 participants
@darkweaver87 @mloiseleur @jnoordsij @genebean @traefiker