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

fix: not generate selector of deployment/daemonset based on the custom label configuration of EnvoyProxy #3995

Merged
merged 4 commits into from
Aug 14, 2024

Conversation

sanposhiho
Copy link
Contributor

@sanposhiho sanposhiho commented Aug 3, 2024

What type of PR is this?

What this PR does / why we need it:

The selector on Deployment and Daemonset is immutable.
We currently generate the selector based on a custom label config of envoy proxy (i.e., the input from users) and users can change the config anytime.
So, if users make such changes, the reconciliation tries to update the selector too, and fails (#1818 #3666).

This PR makes the selector of Deployment and Daemonset stable, generating it without the custom labels.
But, if we just do that, it could result in the breaking change to existing users who already has a deployment / daemonset with a custom label (because the envoy gateway of new version tries to eliminate the custom label from the selector)

So, in order not to introduce such breaking changes, this PR tries to always keep the selector of existing deployment or daemonset.
So, all new deployment and daemonset will have a selector generated without the custom labels, and all existing ones will keep a selector generated with or without the custom labels.
It also means we only help future users; we don't help existing users who already have a deployment/daemonset created with a custom label and want to change the custom label. They should recreate envoyproxy by themselves to recreate the underlying deployment/daemonset. (We don't implement such recreation because it's not a fair deal to implement/maintain a complicated safe recreation process for such a few users.)

Which issue(s) this PR fixes:

Fixes #1818 #3666

@sanposhiho sanposhiho requested a review from a team as a code owner August 3, 2024 04:41
…m label configuration of EnvoyProxy

Signed-off-by: Kensei Nakada <handbomusic@gmail.com>
Signed-off-by: Kensei Nakada <handbomusic@gmail.com>
@sanposhiho sanposhiho force-pushed the custom-label-selector branch 2 times, most recently from 62ace99 to d6eb178 Compare August 3, 2024 06:00
Copy link

codecov bot commented Aug 3, 2024

Codecov Report

Attention: Patch coverage is 80.95238% with 8 lines in your changes missing coverage. Please review.

Project coverage is 67.66%. Comparing base (bd26a41) to head (5ef25a5).
Report is 6 commits behind head on main.

Files Patch % Lines
...ternal/infrastructure/kubernetes/infra_resource.go 74.19% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3995      +/-   ##
==========================================
+ Coverage   67.61%   67.66%   +0.05%     
==========================================
  Files         186      186              
  Lines       22757    22785      +28     
==========================================
+ Hits        15387    15418      +31     
+ Misses       6266     6263       -3     
  Partials     1104     1104              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Kensei Nakada <handbomusic@gmail.com>
@sanposhiho sanposhiho force-pushed the custom-label-selector branch from d6eb178 to d86df6a Compare August 3, 2024 06:16
@zirain
Copy link
Contributor

zirain commented Aug 4, 2024

/retest

@arkodg arkodg requested a review from a team August 13, 2024 00:34
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg requested review from a team August 13, 2024 18:08
@arkodg arkodg added this to the v1.2.0-rc1 milestone Aug 13, 2024
@arkodg arkodg added the area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. label Aug 13, 2024
@zirain
Copy link
Contributor

zirain commented Aug 14, 2024

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify EnvoyProxy podLabels causes reconcilation errors
3 participants