-
Notifications
You must be signed in to change notification settings - Fork 193
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: upgrade hook job npe #8780
base: release-0.9
Are you sure you want to change the base?
fix: upgrade hook job npe #8780
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release-0.9 #8780 +/- ##
===============================================
+ Coverage 64.56% 64.66% +0.09%
===============================================
Files 359 359
Lines 46301 46301
===============================================
+ Hits 29896 29941 +45
+ Misses 13725 13692 -33
+ Partials 2680 2668 -12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
func IgnoreKubeblocksHook(ctx context.Context, client *kubernetes.Clientset, ns string) bool { | ||
deploy, err := GetKubeBlocksDeploy(ctx, client, ns, kubeblocksAppComponent) | ||
if err != nil { | ||
return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hook will continue running if an error occurs (e.g. network errors). Is it an expected behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is expected. If an error occurs, it should be handled by hook job. The hook job may be processed successfully, or it may fail and try again.
The only case in which upgrading can be ignored: the kubeblocks component is not deployed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but if a network error occurs, you will not know whether the deployment exists. It would be better to return this error to main() and exit quickly.
No description provided.