-
Notifications
You must be signed in to change notification settings - Fork 440
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 status for tenant upon binary update failure #272
Conversation
Signed-off-by: soulseen <zhuxiaoyang1996@gmail>
This PR also borrows the change from #260
} | ||
|
||
if us.CurrentVersion != us.UpdatedVersion { | ||
klog.Infof("Tenant '%s' MinIO updated successfully from: %s, to: %s successfully", | ||
name, us.CurrentVersion, us.UpdatedVersion) | ||
} else { | ||
klog.Infof("Tenant '%s' MinIO is already running the most recent version of %s", | ||
msg := fmt.Sprintf("Tenant '%s' MinIO is already running the most recent version of %s", |
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.
but if we do this, we are never going back to ready state are we?
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.
@dvaldivia yes we are never ready if update failed.
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.
makes sense, but it's not an static status state, I think we can improve the status in the future to have status + message so we can provide details on why we reached a certain state
} | ||
|
||
if us.CurrentVersion != us.UpdatedVersion { | ||
klog.Infof("Tenant '%s' MinIO updated successfully from: %s, to: %s successfully", | ||
name, us.CurrentVersion, us.UpdatedVersion) | ||
} else { | ||
klog.Infof("Tenant '%s' MinIO is already running the most recent version of %s", | ||
msg := fmt.Sprintf("Tenant '%s' MinIO is already running the most recent version of %s", |
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.
makes sense, but it's not an static status state, I think we can improve the status in the future to have status + message so we can provide details on why we reached a certain state
This PR also borrows the change from #260