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(lifecycle-operator): introduce separate controller for removing scheduling gates from pods #2946

Merged
merged 37 commits into from
Feb 7, 2024

Conversation

bacherfl
Copy link
Member

@bacherfl bacherfl commented Feb 2, 2024

Closes #2937

This PR introduces a dedicated controller that takes care of removing the scheduling gates of a pod for which there is a related KeptnWorkloadVersion that has passed the pre deployment phase.

This might be the more robust solution, as opposed to the one proposed in the issue description, as checking for completed KeptnWorkloadVersions for a pod in the mutating webhook might lead to some timing issues (e.g. when a KeptnWorkload is finished during the timeframe between adding the scheduling gate in the webhook and the completion of the webhook where the scheduling gate is actually applied).

Note that for pods where the checks of the related KeptnWorkloadVersion have already passed, the controller will just remove the scheduling gate from the pods - pre/post deployment tasks which have already been executed for the WorkloadVersion will not be executed again

Copy link

codecov bot commented Feb 2, 2024

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (882b442) 52.85% compared to head (cea1a33) 85.77%.
Report is 20 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2946       +/-   ##
===========================================
+ Coverage   52.85%   85.77%   +32.92%     
===========================================
  Files          22      162      +140     
  Lines        1771    10351     +8580     
===========================================
+ Hits          936     8879     +7943     
- Misses        740     1184      +444     
- Partials       95      288      +193     
Files Coverage Δ
...cle-operator/controllers/common/helperfunctions.go 98.41% <100.00%> (ø)
.../controllers/lifecycle/keptnworkload/controller.go 82.41% <ø> (+18.68%) ⬆️
...llers/lifecycle/keptnworkloadversion/controller.go 90.31% <66.66%> (+26.35%) ⬆️
...ontrollers/lifecycle/schedulinggates/controller.go 67.18% <67.18%> (ø)

... and 147 files with indirect coverage changes

Flag Coverage Δ
certificate-operator 67.42% <ø> (?)
component-tests 56.73% <66.66%> (+0.93%) ⬆️
lifecycle-operator 85.28% <63.85%> (?)
metrics-operator 87.63% <ø> (?)
scheduler 36.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@github-actions github-actions bot added the ops label Feb 5, 2024
@bacherfl bacherfl changed the title fix(lifecycle-operator): trigger WorkloadVersion reconciliation after changes to related pod fix(lifecycle-operator): introduce separate controller for removing scheduling gates from pods Feb 5, 2024
eddycharly and others added 26 commits February 5, 2024 13:38
Signed-off-by: Charles-Edouard Brétéché <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Charles-Edouard Brétéché <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
…nsistent with KeptnWorkload

Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
… changes of related pod

Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Charles-Edouard Brétéché <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: RealAnna <[email protected]>
Signed-off-by: RealAnna <[email protected]>
Co-authored-by: Florian Bacher <[email protected]>
Co-authored-by: Meg McRoberts <[email protected]>
Co-authored-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
…n#2915)

Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: check-spelling-bot <[email protected]>
Co-authored-by: Moritz Wiesinger <[email protected]>
Co-authored-by: Meg McRoberts <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
)

Signed-off-by: Meg McRoberts <[email protected]>
Co-authored-by: odubajDT <[email protected]>
Co-authored-by: Florian Bacher <[email protected]>
Co-authored-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
…e for re-reconciliation

Signed-off-by: Florian Bacher <[email protected]>
@bacherfl bacherfl force-pushed the fix/2937/restart-pod branch from fadbd2e to 4b65576 Compare February 5, 2024 12:39
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 5, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 5, 2024
@bacherfl bacherfl marked this pull request as ready for review February 5, 2024 12:55
@bacherfl bacherfl requested a review from a team as a code owner February 5, 2024 12:55
@RealAnna
Copy link
Contributor

RealAnna commented Feb 5, 2024

would it make sense to recreate the issue with a kuttl/chainsaw test?

having a deployment annotated for keptn with a simple pre task. After it passes delete the pod and check 1- it gets gated 2- the gate is removed?

Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
RealAnna
RealAnna previously approved these changes Feb 6, 2024
odubajDT
odubajDT previously approved these changes Feb 7, 2024
Copy link
Contributor

@odubajDT odubajDT left a comment

Choose a reason for hiding this comment

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

left small comment, but leaving it up to you :)

Signed-off-by: Florian Bacher <[email protected]>
@bacherfl bacherfl dismissed stale reviews from odubajDT and RealAnna via cea1a33 February 7, 2024 07:57
Copy link

sonarqubecloud bot commented Feb 7, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@bacherfl bacherfl merged commit 9fa3770 into keptn:main Feb 7, 2024
38 checks passed
@bacherfl bacherfl deleted the fix/2937/restart-pod branch February 7, 2024 08:18
Bharadwajshivam28 pushed a commit to Bharadwajshivam28/lifecycle-toolkit that referenced this pull request Feb 20, 2024
…cheduling gates from pods (keptn#2946)

Signed-off-by: Charles-Edouard Brétéché <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: RealAnna <[email protected]>
Signed-off-by: RealAnna <[email protected]>
Signed-off-by: check-spelling-bot <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Co-authored-by: Charles-Edouard Brétéché <[email protected]>
Co-authored-by: Moritz Wiesinger <[email protected]>
Co-authored-by: RealAnna <[email protected]>
Co-authored-by: Meg McRoberts <[email protected]>
Co-authored-by: odubajDT <[email protected]>
Signed-off-by: shivam <[email protected]>
Vickysomtee pushed a commit to Vickysomtee/keptn-lifecycle-toolkit that referenced this pull request Apr 23, 2024
…cheduling gates from pods (keptn#2946)

Signed-off-by: Charles-Edouard Brétéché <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: RealAnna <[email protected]>
Signed-off-by: RealAnna <[email protected]>
Signed-off-by: check-spelling-bot <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Co-authored-by: Charles-Edouard Brétéché <[email protected]>
Co-authored-by: Moritz Wiesinger <[email protected]>
Co-authored-by: RealAnna <[email protected]>
Co-authored-by: Meg McRoberts <[email protected]>
Co-authored-by: odubajDT <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pods cannot be restarted when using scheduling gates
6 participants