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

gateway-api: Merge externally annotations and labels for kubernetes types #27251

Merged
merged 1 commit into from Aug 16, 2023
Merged

gateway-api: Merge externally annotations and labels for kubernetes types #27251

merged 1 commit into from Aug 16, 2023

Conversation

farodin91
Copy link
Contributor

@farodin91 farodin91 commented Aug 3, 2023 •
edited

We are currently using Metallb and in combination with Cilium. Metallb is adding automatically annotations labels for the selected pools and the gateway reconciler did remove these annotations again.

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Fixes: #24584

gateway-api: Merge externally annotations and labels for kubernetes types

@farodin91 farodin91 requested a review from a team as a code owner August 3, 2023 14:50
@farodin91 farodin91 requested a review from meyskens August 3, 2023 14:50
@maintainer-s-little-helper
Copy link

Commit fca424b33583ffff2bcb3e23963d57c31945a7ad does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Aug 3, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Aug 3, 2023
@farodin91 farodin91 force-pushed the allow-pre-existing-annotations-for-gateways branch from fca424b to ef39e4e Compare August 3, 2023 14:51
@maintainer-s-little-helper
Copy link

Commit fca424b33583ffff2bcb3e23963d57c31945a7ad does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@farodin91 farodin91 force-pushed the allow-pre-existing-annotations-for-gateways branch from ef39e4e to 63c1ca3 Compare August 3, 2023 14:56
@farodin91 farodin91 requested a review from a team as a code owner August 3, 2023 14:56
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Aug 3, 2023
@farodin91 farodin91 force-pushed the allow-pre-existing-annotations-for-gateways branch from 63c1ca3 to e52409c Compare August 3, 2023 14:58
@farodin91 farodin91 changed the title allow pre existing annotations for gateways gateway-api: Allow externally managed annotations for services Aug 3, 2023
@dylandreimerink dylandreimerink added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. labels Aug 4, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Aug 4, 2023
@dylandreimerink
Copy link
Member

/test

meyskens
meyskens reviewed Aug 4, 2023
View reviewed changes
USERS.md Outdated Show resolved Hide resolved
meyskens
meyskens reviewed Aug 4, 2023
View reviewed changes
Copy link
Member

@meyskens meyskens left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! Just some small fixes needed to get it approved :)

operator/pkg/gateway-api/gateway_reconcile.go Outdated Show resolved Hide resolved
@farodin91 farodin91 force-pushed the allow-pre-existing-annotations-for-gateways branch 2 times, most recently from 94a590c to 740add6 Compare August 4, 2023 13:25
@farodin91
Copy link
Contributor Author

@meyskens Updated. Will this be backport to 1.14?

@sayboras sayboras added needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.1 Aug 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.6 Aug 6, 2023
@sayboras sayboras added area/servicemesh GH issues or PRs regarding servicemesh feature/k8s-gateway-api labels Aug 6, 2023
@farodin91
Copy link
Contributor Author

@farodin91 please shorten the commit's subject:


 Error: ERROR:CUSTOM: Please avoid long commit subjects (max: 75, found: 93)

Any good idea?

@farodin91 farodin91 force-pushed the allow-pre-existing-annotations-for-gateways branch from 691576b to 28247e0 Compare August 8, 2023 06:50
@farodin91
Copy link
Contributor Author

@ldelossa i have updated the commit message

@farodin91 farodin91 changed the title gateway-api: Allow externally managed annotations for services gateway-api: Merge externally annotations and labels for kubernetes types Aug 9, 2023
@nebril nebril added this to Needs backport from main in 1.13.7 Aug 10, 2023
@nebril nebril removed this from Needs backport from main in 1.13.6 Aug 10, 2023
@farodin91
gateway-api: Merge externally annotations and labels for kubernetes t…
8faca30 
…ypes

Fixes cilium#24584

Signed-off-by: Jan Jansen <jan.jansen@gdata.de>
@farodin91 farodin91 force-pushed the allow-pre-existing-annotations-for-gateways branch from 28247e0 to 8faca30 Compare August 10, 2023 13:07
@farodin91
Copy link
Contributor Author

@sayboras rebased and updated commit message.

@nebril nebril added this to Needs backport from main in 1.14.2 Aug 10, 2023
@nebril nebril removed this from Needs backport from main in 1.14.1 Aug 10, 2023
@sayboras
Copy link
Member

/test

@ti-mo ti-mo merged commit ec05fd4 into cilium:main Aug 16, 2023
59 checks passed
@tklauser tklauser mentioned this pull request Aug 22, 2023
Merged
22 tasks
@tklauser tklauser added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 22, 2023
@tklauser tklauser mentioned this pull request Aug 23, 2023
Merged
8 tasks
@tklauser tklauser added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Aug 23, 2023
@farodin91 farodin91 deleted the allow-pre-existing-annotations-for-gateways branch August 25, 2023 11:28
@joestringer joestringer added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Aug 25, 2023
@joestringer joestringer moved this from Needs backport from main to Backport done to v1.13 in 1.13.7 Aug 25, 2023
@joestringer joestringer moved this from Needs backport from main to Backport done to v1.14 in 1.14.2 Aug 25, 2023
This was referenced Sep 9, 2023
Merged
Merged
@PhilipSchmid PhilipSchmid mentioned this pull request Sep 27, 2023
Closed
2 tasks
@zs-ko zs-ko mentioned this pull request Oct 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meyskens meyskens meyskens left review comments

@sayboras sayboras sayboras approved these changes

Assignees
No one assigned
Labels
area/servicemesh GH issues or PRs regarding servicemesh backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. feature/k8s-gateway-api kind/bug This is a bug in the Cilium logic. kind/community-contribution This was a contribution made by a community member. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.13.7
Backport done to v1.13
1.14.2
Backport done to v1.14
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Log Explosion with Gateway API Enabled + Gateway Resource
8 participants
@farodin91 @dylandreimerink @sayboras @ldelossa @meyskens @tklauser @joestringer @ti-mo