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

Update deprecated Objects only if they exist #1540

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

cniackz
Copy link
Contributor

@cniackz cniackz commented Mar 31, 2023

Objective:

To update objects only if they exist. When an object exist, there is no error.

To fix: #1530

pkg/controller/upgrades.go Outdated Show resolved Hide resolved
@harshavardhana
Copy link
Member

I think the correct fix is to do this

diff --git a/pkg/controller/upgrades.go b/pkg/controller/upgrades.go
index 176790f5..e3f33f2a 100644
--- a/pkg/controller/upgrades.go
+++ b/pkg/controller/upgrades.go
@@ -332,7 +332,7 @@ func (c *Controller) upgrade500(ctx context.Context, tenant *miniov2.Tenant) (*m
        if err != nil && !k8serrors.IsNotFound(err) {
                return tenant, err
        }
-       if logSearchSecret != nil {
+       if err == nil && logSearchSecret != nil {
                logSearchSecret.ObjectMeta.OwnerReferences = nil
                if _, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Update(ctx, logSearchSecret, metav1.UpdateOptions{}); err != nil {
                        return tenant, err
@@ -343,7 +343,7 @@ func (c *Controller) upgrade500(ctx context.Context, tenant *miniov2.Tenant) (*m
        if err != nil && !k8serrors.IsNotFound(err) {
                return tenant, err
        }
-       if logSearchAPISvc != nil {
+       if err == nil && logSearchAPISvc != nil {
                logSearchAPISvc.ObjectMeta.OwnerReferences = nil
                if _, err = c.kubeClientSet.CoreV1().Services(tenant.Namespace).Update(ctx, logSearchAPISvc, metav1.UpdateOptions{}); err != nil {
                        return tenant, err
@@ -354,7 +354,7 @@ func (c *Controller) upgrade500(ctx context.Context, tenant *miniov2.Tenant) (*m
        if err != nil && !k8serrors.IsNotFound(err) {
                return tenant, err
        }
-       if logPgSS != nil {
+       if err == nil && logPgSS != nil {
                logPgSS.ObjectMeta.OwnerReferences = nil
                if _, err = c.kubeClientSet.AppsV1().StatefulSets(tenant.Namespace).Update(ctx, logPgSS, metav1.UpdateOptions{}); err != nil {
                        return tenant, err
@@ -365,7 +365,7 @@ func (c *Controller) upgrade500(ctx context.Context, tenant *miniov2.Tenant) (*m
        if err != nil && !k8serrors.IsNotFound(err) {
                return tenant, err
        }
-       if logHlSvc != nil {
+       if err == nil && logHlSvc != nil {
                logHlSvc.ObjectMeta.OwnerReferences = nil
                if _, err = c.kubeClientSet.CoreV1().Services(tenant.Namespace).Update(ctx, logHlSvc, metav1.UpdateOptions{}); err != nil {
                        return tenant, err
@@ -376,7 +376,7 @@ func (c *Controller) upgrade500(ctx context.Context, tenant *miniov2.Tenant) (*m
        if err != nil && !k8serrors.IsNotFound(err) {
                return tenant, err
        }
-       if promCM != nil {
+       if err == nil && promCM != nil {
                promCM.ObjectMeta.OwnerReferences = nil
                if _, err = c.kubeClientSet.CoreV1().ConfigMaps(tenant.Namespace).Update(ctx, promCM, metav1.UpdateOptions{}); err != nil {
                        return tenant, err
@@ -387,7 +387,7 @@ func (c *Controller) upgrade500(ctx context.Context, tenant *miniov2.Tenant) (*m
        if err != nil && !k8serrors.IsNotFound(err) {
                return tenant, err
        }
-       if promHlSvc != nil {
+       if err == nil && promHlSvc != nil {
                promHlSvc.ObjectMeta.OwnerReferences = nil
                if _, err = c.kubeClientSet.CoreV1().Services(tenant.Namespace).Update(ctx, promHlSvc, metav1.UpdateOptions{}); err != nil {
                        return tenant, err
@@ -398,7 +398,7 @@ func (c *Controller) upgrade500(ctx context.Context, tenant *miniov2.Tenant) (*m
        if err != nil && !k8serrors.IsNotFound(err) {
                return tenant, err
        }
-       if prometheusStatefulSet != nil {
+       if err == nil && prometheusStatefulSet != nil {
                prometheusStatefulSet.ObjectMeta.OwnerReferences = nil
                if _, err = c.kubeClientSet.AppsV1().StatefulSets(tenant.Namespace).Update(ctx, prometheusStatefulSet, metav1.UpdateOptions{}); err != nil {
                        return tenant, err

pkg/controller/upgrades.go Outdated Show resolved Hide resolved
@cniackz cniackz force-pushed the logSearchSecret-fix branch 2 times, most recently from 1d61e55 to 2fab667 Compare March 31, 2023 22:27
@cniackz
Copy link
Contributor Author

cniackz commented Mar 31, 2023

@harshavardhana having logSearchSecret != nil is not good because it won't never be nil even if the secret does not exist. So we need to check for the error to see if object exist or not.

@cniackz
Copy link
Contributor Author

cniackz commented Mar 31, 2023

Never mind, I see your point, and your solution is correct because even if k8s function changes, we are still protected and only update if object is not nil, cool!...

@cniackz cniackz dismissed harshavardhana’s stale review March 31, 2023 22:39

Changed what was requested, please review again

@cniackz cniackz changed the title Update resource only if it exist Update Objects only if they exist Mar 31, 2023
Copy link
Contributor

@allanrogerr allanrogerr left a comment

Choose a reason for hiding this comment

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

Logical change and modification

@harshavardhana harshavardhana changed the title Update Objects only if they exist Update deprecated Objects only if they exist Mar 31, 2023
@harshavardhana harshavardhana merged commit 5f0856d into minio:master Mar 31, 2023
24 checks passed
@cniackz cniackz deleted the logSearchSecret-fix branch April 1, 2023 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in Operator logs after upgrading to 5.0.0: resource name may not be empty
4 participants