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

refactor(main): skip success exec #4162

Merged
merged 1 commit into from Oct 26, 2023

Conversation

cuisongliu
Copy link
Collaborator

@cuisongliu cuisongliu commented Oct 24, 2023

🤖 Generated by Copilot at 0264836

Summary

🐛🚫🚀

This pull request fixes a bug in the apply command that would report success even if there were no resources to apply or if the apply failed. It changes the updateStatus function in pkg/apply/applydrivers/apply_drivers_default.go to return early if there are no new images, and removes the unused NewSuccessCommandCondition function from pkg/types/v1beta1/cluster.go.

applydrivers fix
no success if nothing ran
autumn bug hunting

Walkthrough

  • Add a return statement to avoid updating the command condition with a success status and the new images if the apply command did not run any resources on the cluster (link)
  • Remove the unused helper function NewSuccessCommandCondition that created a command condition with a success status and a message for the apply command (link)

Signed-off-by: cuisongliu <cuisongliu@qq.com>
@cuisongliu cuisongliu added this to the v4.4 milestone Oct 24, 2023
@sealos-ci-robot
Copy link
Member

🤖 Generated by lychee action

Summary

Status Count
🔍 Total 956
✅ Successful 375
⏳ Timeouts 0
🔀 Redirected 0
👻 Excluded 580
❓ Unknown 0
🚫 Errors 0

Full action output

Full Github Actions output

@sweep-ai
Copy link

sweep-ai bot commented Oct 24, 2023

Apply Sweep Rules to your PR?

  • Apply: Leftover TODOs in the code should be handled.
  • Apply: All new business logic should have corresponding unit tests in the tests/ directory.
  • Apply: Any clearly inefficient or repeated code should be optimized or refactored.

@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!.

@cuisongliu cuisongliu merged commit 94c8009 into labring:main Oct 26, 2023
97 checks passed
@sealos-ci-robot
Copy link
Member

/cherry-pick release-v4.4

@sealos-ci-robot
Copy link
Member

🤖 says: cherry pick action finished successfully 🎉!
See: https://github.com/labring/sealos/actions/runs/6648725051

sealos-ci-robot pushed a commit that referenced this pull request Oct 26, 2023
cuisongliu added a commit that referenced this pull request Oct 26, 2023
Co-authored-by: cuisongliu <cuisongliu@qq.com>
sealos-ci-robot added a commit that referenced this pull request Oct 26, 2023
Co-authored-by: cuisongliu <cuisongliu@qq.com>
cuisongliu added a commit that referenced this pull request Oct 26, 2023
Co-authored-by: cuisongliu <cuisongliu@qq.com>
@cuisongliu cuisongliu deleted the skip_save_success branch October 26, 2023 10:32
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.

None yet

3 participants