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

ingress: Propagate required annotations from Ingress to LB Service #20860

Merged
merged 1 commit into from
Aug 22, 2022
Merged

ingress: Propagate required annotations from Ingress to LB Service #20860

merged 1 commit into from
Aug 22, 2022

Conversation

NikhilSharmaWe
Copy link
Contributor

This PR adds the required annotations from Ingress to Load Balancer Service.
Fixes: #20646

@NikhilSharmaWe NikhilSharmaWe requested a review from a team as a code owner August 10, 2022 17:30
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 10, 2022
@NikhilSharmaWe NikhilSharmaWe force-pushed the lbSerAnnotations branch 3 times, most recently from eaf8431 to 99e84de Compare August 10, 2022 17:37
@sayboras sayboras added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Aug 12, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 12, 2022
@sayboras sayboras added the area/operator Impacts the cilium-operator component label Aug 12, 2022
@sayboras sayboras changed the title propogate required annotations from Ingress to Load Balancer Service ingress: Propagate required annotations from Ingress to LB Service Aug 12, 2022
@sayboras sayboras self-requested a review August 12, 2022 07:21
sayboras
sayboras reviewed Aug 12, 2022
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

I have couple of comments as per below. If you are still working on this, feel free to mark PR as draft, so that other reviewers are aware.

operator/pkg/ingress/service.go Outdated Show resolved Hide resolved
@NikhilSharmaWe NikhilSharmaWe force-pushed the lbSerAnnotations branch 4 times, most recently from 1b322d9 to 311fd78 Compare August 12, 2022 08:46
aanm
aanm reviewed Aug 12, 2022
View reviewed changes
operator/pkg/ingress/service.go Outdated Show resolved Hide resolved
operator/pkg/ingress/service.go Outdated Show resolved Hide resolved
@NikhilSharmaWe NikhilSharmaWe requested review from a team as code owners August 12, 2022 11:16
@NikhilSharmaWe NikhilSharmaWe force-pushed the lbSerAnnotations branch 4 times, most recently from e230170 to 689ac87 Compare August 12, 2022 12:16
aanm
aanm requested changes Aug 12, 2022
View reviewed changes
operator/option/config.go Outdated Show resolved Hide resolved
@NikhilSharmaWe NikhilSharmaWe force-pushed the lbSerAnnotations branch 2 times, most recently from 5b27c73 to 0697bdf Compare August 12, 2022 14:36
@sayboras
Copy link
Member

CI was passed before, the force pushes are just to adjust commit message (e.g. typo and sign off).

Mark this ready to merge.

@sayboras sayboras added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 20, 2022
@sayboras sayboras removed the request for review from a team August 20, 2022 13:46
@aditighag aditighag merged commit c9ccf17 into cilium:master Aug 22, 2022
@aditighag aditighag mentioned this pull request Aug 23, 2022
Merged
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Needs backport from master in 1.12.2 Aug 24, 2022
@sayboras sayboras mentioned this pull request Sep 7, 2022
Merged
sayboras added a commit to sayboras/cilium that referenced this pull request Sep 7, 2022
@sayboras
ingress: Rename LB annotation to annotation prefixes
9e2bf33 
This is to avoid any potential confusion from user. Also, related
docs are updated.

Related: cilium#20860

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.12.2 Sep 7, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Needs backport from master in 1.12.2 Sep 7, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.12.2 Sep 7, 2022
@sayboras sayboras mentioned this pull request Sep 7, 2022
Merged
nebril pushed a commit that referenced this pull request Sep 9, 2022
@sayboras @nebril
ingress: Rename LB annotation to annotation prefixes
481493b 
This is to avoid any potential confusion from user. Also, related
docs are updated.

Related: #20860

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to sayboras/cilium that referenced this pull request Sep 9, 2022
@sayboras
ingress: Rename LB annotation to annotation prefixes
21a6aea 
[ upstream commit 481493b ]

This is to avoid any potential confusion from user. Also, related
docs are updated.

Related: cilium#20860

Signed-off-by: Tam Mach <tam.mach@cilium.io>
nebril pushed a commit that referenced this pull request Sep 13, 2022
@sayboras @nebril
ingress: Rename LB annotation to annotation prefixes
d713831 
[ upstream commit 481493b ]

This is to avoid any potential confusion from user. Also, related
docs are updated.

Related: #20860

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@nebril nebril added backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed needs-backport/1.12 labels Sep 13, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport done to v1.12 in 1.12.2 Sep 13, 2022
@nebril nebril mentioned this pull request Sep 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aanm aanm aanm approved these changes

@sayboras sayboras sayboras approved these changes

@qmonnet qmonnet qmonnet approved these changes

Assignees
No one assigned
Labels
area/operator Impacts the cilium-operator component backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.12.2
Backport done to v1.12
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CFP: IngressController configurable cloud provider related LoadBalancer
6 participants
@NikhilSharmaWe @aanm @sayboras @qmonnet @nebril @aditighag