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

add a flag to select target types to deregister instance from #214

Merged
merged 10 commits into from
Sep 9, 2024

Conversation

yuxuanzhu0214
Copy link
Contributor

@yuxuanzhu0214 yuxuanzhu0214 commented Aug 21, 2024

Changes
Added a flag deregister-target-types which is a comma separated list of target types to deregister instance from. By default the value is "classic-elb,target-group". Supported target types are classic-elb and target-group.

Tests
Manually applied the image in dev cluster.

When --deregister-target-types=classic-elb,target-group, life cycle manager scan through all targets that match both target types.

time="2024-08-21T23:21:49Z" level=info msg="i-0ae3409fc1896603a> scanner starting"
time="2024-08-21T23:21:50Z" level=debug msg="cache hit => false, service => elasticloadbalancing.DescribeTargetGroups"
time="2024-08-21T23:21:50Z" level=debug msg="cache hit => false, service => elasticloadbalancing.DescribeLoadBalancers"
time="2024-08-21T23:21:50Z" level=info msg="i-0ae3409fc1896603a> checking targetgroup/elb membership"
time="2024-08-21T23:21:50Z" level=debug msg="len targetgroup: 42, len elb: 1"

When - --deregister-target-types=classic-elb, life cycle manager only scan through targets that are classic elb.

time="2024-08-21T23:08:44Z" level=info msg="i-03fffc250f0ffbe14> scanner starting"
time="2024-08-21T23:08:44Z" level=debug msg="cache hit => true, service => elasticloadbalancing.DescribeLoadBalancers"
time="2024-08-21T23:08:44Z" level=info msg="i-03fffc250f0ffbe14> checking targetgroup/elb membership"
time="2024-08-21T23:08:44Z" level=debug msg="len targetgroup: 0, len elb: 1"

Then life cycle manager deregisters the instance only from targets found from scanning.

@yuxuanzhu0214 yuxuanzhu0214 requested review from a team as code owners August 21, 2024 23:34
Yuxuan Zhu added 5 commits August 21, 2024 16:51
Signed-off-by: Yuxuan Zhu <[email protected]>
Signed-off-by: Yuxuan Zhu <[email protected]>
Signed-off-by: Yuxuan Zhu <[email protected]>
Signed-off-by: Yuxuan Zhu <[email protected]>
Copy link

@estela-ramirez estela-ramirez left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

codecov bot commented Aug 24, 2024

Codecov Report

Attention: Patch coverage is 68.75000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 66.72%. Comparing base (ad9b7d7) to head (c80a669).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
pkg/service/server.go 68.75% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #214      +/-   ##
==========================================
+ Coverage   66.52%   66.72%   +0.19%     
==========================================
  Files          15       15              
  Lines        1168     1172       +4     
==========================================
+ Hits          777      782       +5     
+ Misses        319      318       -1     
  Partials       72       72              

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

Dockerfile Outdated Show resolved Hide resolved
Signed-off-by: Yuxuan Zhu <[email protected]>
@tekenstam tekenstam enabled auto-merge (squash) September 9, 2024 23:17
@tekenstam tekenstam merged commit f3b1488 into keikoproj:master Sep 9, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants