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: remove old workflows on addon checksum change #297

Merged
merged 3 commits into from
Oct 30, 2023

Conversation

ccpeng
Copy link
Contributor

@ccpeng ccpeng commented Oct 28, 2023

fixes: #285 #174

with contribution from @estela-ramirez

a new addon (but has the same checksum from recent past) won't generate a new workflow since a previous workflow was already created; as a result, the addon gets stuck on pending

fix is to purge old workflows so a new workflow can get created

@ccpeng ccpeng force-pushed the repeated-addon-stale-wf branch from d5f5b80 to ba9998b Compare October 28, 2023 22:12
@codecov
Copy link

codecov bot commented Oct 28, 2023

Codecov Report

Merging #297 (9ebe761) into master (ef27eb5) will decrease coverage by 0.39%.
The diff coverage is 64.28%.

@@            Coverage Diff             @@
##           master     #297      +/-   ##
==========================================
- Coverage   56.20%   55.81%   -0.39%     
==========================================
  Files          14       14              
  Lines        1893     1933      +40     
==========================================
+ Hits         1064     1079      +15     
- Misses        715      734      +19     
- Partials      114      120       +6     
Files Coverage Δ
pkg/workflows/workflow.go 68.71% <100.00%> (+0.09%) ⬆️
controllers/addon_controller.go 53.53% <63.41%> (-1.72%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ccpeng ccpeng force-pushed the repeated-addon-stale-wf branch from ba9998b to d81312e Compare October 28, 2023 22:39
Signed-off-by: Chun-Che Peng <chunche.peng@gmail.com>
@ccpeng ccpeng force-pushed the repeated-addon-stale-wf branch from d81312e to 206d8c1 Compare October 28, 2023 22:40
Signed-off-by: Chun-Che Peng <chunche.peng@gmail.com>
@ccpeng ccpeng assigned ccpeng and unassigned ccpeng Oct 29, 2023
@ccpeng ccpeng marked this pull request as ready for review October 29, 2023 06:39
@ccpeng ccpeng requested a review from a team as a code owner October 29, 2023 06:39
@ccpeng ccpeng changed the title draft: Repeated addon stale wf fix: remove old workflows on addon checksum change Oct 29, 2023
@ccpeng ccpeng linked an issue Oct 29, 2023 that may be closed by this pull request
kevdowney
kevdowney previously approved these changes Oct 30, 2023
Copy link
Collaborator

@kevdowney kevdowney left a comment

Choose a reason for hiding this comment

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

LGTM

@kevdowney
Copy link
Collaborator

I think the Addon can still get stuck in pending if there's no further updates or controllers aren't updating... we can additionally add a check before return from

return ctrl.Result{}, nil

if instance.IsPending() {
  return ctrl.Result{RequeAfter: 5 * Duration.Seconds}, nil
}

ZihanJiang96
ZihanJiang96 previously approved these changes Oct 30, 2023
Signed-off-by: Chun-Che Peng <chunche.peng@gmail.com>
@ccpeng ccpeng dismissed stale reviews from ZihanJiang96 and kevdowney via 9ebe761 October 30, 2023 21:30
Copy link
Collaborator

@kevdowney kevdowney left a comment

Choose a reason for hiding this comment

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

LGTM

@ccpeng ccpeng merged commit e54d5ca into keikoproj:master Oct 30, 2023
6 of 7 checks passed
@ccpeng ccpeng deleted the repeated-addon-stale-wf branch October 30, 2023 21:57
@ccpeng ccpeng mentioned this pull request Oct 30, 2023
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.

bug: Addon status stuck in pending feat: Purge old generated workflows
3 participants