Skip to content

Optimize and fix some issues#2

Open
coldzerofear wants to merge 1 commit into
harvester:mainfrom
coldzerofear:fix
Open

Optimize and fix some issues#2
coldzerofear wants to merge 1 commit into
harvester:mainfrom
coldzerofear:fix

Conversation

@coldzerofear

Copy link
Copy Markdown

Problem:

Solution:

Related Issue(s):

Test plan:

Additional documentation or context

annotations:
"helm.sh/hook": "pre-install"
"helm.sh/hook-delete-policy": "before-hook-creation"
"helm.sh/hook-weight": "5"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is to apply the new CA when performing an upgrade

// check and add finalizer
if !controllerutil.ContainsFinalizer(config, kubeovniov1.KubeOVNConfigurationFinalizer) {
controllerutil.AddFinalizer(config, kubeovniov1.KubeOVNConfigurationFinalizer)
if controllerutil.AddFinalizer(config, kubeovniov1.KubeOVNConfigurationFinalizer) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Delete unnecessary conditional judgments

UpdateFunc: func(e event.TypedUpdateEvent[client.Object]) bool {
return !reflect.DeepEqual(e.ObjectOld.GetLabels(), e.ObjectNew.GetLabels())
},
}))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ensure that reconciliation can be performed when node changes occur

Comment thread internal/render/render.go Outdated
// patch version supplied by the controller
config.Spec.Global.Images.KubeOVNImage.Tag = version
if len(config.Spec.Global.Images.KubeOVNImage.Tag) == 0 {
config.Spec.Global.Images.KubeOVNImage.Tag = version

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

When the tag is explicitly specified in the CR, the specified one should be used first

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

current iteration does not allow tag override since there may be some changes needed to the charts template and updating the tag may not work

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

current iteration does not allow tag override since there may be some changes needed to the charts template and updating the tag may not work

Ok, I will retract the changes made here

{{- if not .Values.disableModulesManagement }}
- SYS_MODULE
{{- end }}
- NET_RAW

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fix possible permission issues

return ctrl.Result{}, nil
} else if err != nil {
r.Log.WithValues("name", req.Name).Error(err, "error fetching object")
return ctrl.Result{}, err

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Get node failed and returned an error to retry, instead of continuing to execute downwards

Signed-off-by: coldzerofear <coldzerofear@outlook.com>
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.

2 participants