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

[bitnami/external-dns] Fix crd.create=true #27434

Merged
merged 10 commits into from
Aug 13, 2024
Merged

Conversation

gnom4ik
Copy link
Contributor

@gnom4ik gnom4ik commented Jun 18, 2024

CRD doesn't work, this solving the problem )

Description of the change

Added annotation that

Benefits

Helm chart will start work if crd.create=true

Possible drawbacks


Applicable issues

  • fixes #

Additional information

This correction adds an annotation to the external-dns CRD. There are no other changes. The consequence of the change is that the chart will start working correctly (currently, enabling the CRD does not result in installation due to the absence of the annotation)."

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

gnom4ik added 2 commits June 18, 2024 15:29
CRD doesn't work, this solving the problem ) 

Signed-off-by: Aleksei Pashkin <[email protected]>
Signed-off-by: Aleksei Pashkin <[email protected]>
@github-actions github-actions bot added external-dns triage Triage is needed labels Jun 18, 2024
@github-actions github-actions bot requested a review from javsalgar June 18, 2024 13:39
@gnom4ik gnom4ik changed the title Fix crd.create=true [bitnami/external-dns] Fix crd.create=true Jun 18, 2024
Signed-off-by: Bitnami Containers <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Jun 18, 2024
@github-actions github-actions bot removed the triage Triage is needed label Jun 18, 2024
@github-actions github-actions bot removed the request for review from javsalgar June 18, 2024 15:33
@github-actions github-actions bot requested a review from fmulero June 18, 2024 15:33
Copy link
Collaborator

@fmulero fmulero left a comment

Choose a reason for hiding this comment

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

Hi @gnom4ik, Thanks a lot for your contribution.

Why is that annotation required? The upstream CRDs don't have that annotation. If the annotation is required beacuase external-dns is using any private/reserved API, it should be communicated to the upstream project to solve the issue.

@fmulero
Copy link
Collaborator

fmulero commented Jun 26, 2024

I've just seen a related PR fixing this problem on external-dns side. Unfortunately there is no new releases with those changes. We should wait for the next release

@jetersen
Copy link
Contributor

Can we not have something that patches it and once the next release comes out it will be patched from their side.
Currently we been waiting over a month for a release from external-dns and several comments on the PR with no success: kubernetes-sigs/external-dns#4488

@fmulero
Copy link
Collaborator

fmulero commented Jul 2, 2024

We could land this change but CRDs are automatically updated and it could be reverted without any advice.

@jetersen
Copy link
Contributor

Can't we fix this in the bitnami bot? 375ee3b (#25962)

Carlos Rodríguez Hernández and others added 2 commits July 18, 2024 12:39
Signed-off-by: Carlos Rodríguez Hernández <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
@juan131
Copy link
Contributor

juan131 commented Jul 22, 2024

Hi @jetersen

I'm afraid we cannot include patches or point to specific commits in our automations to sync changes. Until there's an upstream release, you can use kustomize (or similar tools) to apply the changes in the Bitnami chart, sth like:

helm template external-dns oci://registry-1.docker.io/bitnamicharts/external-dns --set crd.create=true (...) | kubectl apply -k (...)

Copy link

github-actions bot commented Aug 7, 2024

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

@github-actions github-actions bot added stale 15 days without activity and removed stale 15 days without activity labels Aug 7, 2024
@fmulero fmulero added stale 15 days without activity solr and removed solr labels Aug 12, 2024
Signed-off-by: Bitnami Containers <[email protected]>
@github-actions github-actions bot removed the stale 15 days without activity label Aug 13, 2024
juan131
juan131 previously approved these changes Aug 13, 2024
@fmulero fmulero enabled auto-merge (squash) August 13, 2024 11:50
@pschichtel
Copy link

@juan131: Out of interest: Assuming that the next release will definitely include the upstream fix, wouldn't it work just fine including the the annotation now? The bot would update the CRD upon the next release, which would have the change anyway. Or will the bot occasionally verify consistency with the upstream and overwrite the change?

@fmulero
Copy link
Collaborator

fmulero commented Aug 13, 2024

@juan131: Out of interest: Assuming that the next release will definitely include the upstream fix, wouldn't it work just fine including the the annotation now? The bot would update the CRD upon the next release, which would have the change anyway. Or will the bot occasionally verify consistency with the upstream and overwrite the change?

With this change the bot won't know where is the source for that CRD and it won't try to update it. We should add the header again once external-dns is released

@fmulero fmulero merged commit fc11b48 into bitnami:main Aug 13, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external-dns solved verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants