Skip to content

add clusteroperator Upgradeable conditions #206

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 1 commit into from
Feb 18, 2019
Merged

add clusteroperator Upgradeable conditions #206

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions config/v1/types_cluster_operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,18 +112,24 @@ type ClusterOperatorStatusCondition struct {
type ClusterStatusConditionType string

const (
// OperatorAvailable indicates that the binary maintained by the operator (eg: openshift-apiserver for the
// Available indicates that the binary maintained by the operator (eg: openshift-apiserver for the
// openshift-apiserver-operator), is functional and available in the cluster.
OperatorAvailable ClusterStatusConditionType = "Available"

// OperatorProgressing indicates that the operator is actively making changes to the binary maintained by the
// Progressing indicates that the operator is actively making changes to the binary maintained by the
// operator (eg: openshift-apiserver for the openshift-apiserver-operator).
OperatorProgressing ClusterStatusConditionType = "Progressing"

// OperatorFailing indicates that the operator has encountered an error that is preventing it from working properly.
// Failing indicates that the operator has encountered an error that is preventing it from working properly.
// The binary maintained by the operator (eg: openshift-apiserver for the openshift-apiserver-operator) may still be
// available, but the user intent cannot be fulfilled.
OperatorFailing ClusterStatusConditionType = "Failing"

// Upgradeable indicates whether the operator is in a state that is safe to upgrade. When status is `False`
// administrators should not upgrade their cluster and the message field should contain a human readable description
// of what the administrator should do to allow the operator to successfully update. A missing condition, True,
// and Unknown are all treated by the CVO as allowing an upgrade.
OperatorUpgradeable ClusterStatusConditionType = "Upgradeable"
Copy link
Contributor

@bparees bparees Feb 11, 2019

Choose a reason for hiding this comment

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

Since we've already established that it's not necessary an "upgrade", and also because i still think there might be some value in using this to determine general readiness of an operator, i'd propose renaming it to Ready.

Copy link
Contributor Author

@deads2k deads2k Feb 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Since we've already established that it's not necessary an "upgrade", and also because i still think there might be some value in using this to determine general readiness of an operator, i'd propose renaming it to Ready.

You cannot have the CVO base its decision about whether or not to allow a version change upon the operator status for the same reason you cannot allow it make a decision based on operand status. If you did, you would be unable to fix a broken operator with a version change. You're describing a different condition.

The upgradeable condition will block an upgrade and it should only be set in a case when we cannot safely allow a version change to proceed.

Copy link
Contributor

@bparees bparees Feb 11, 2019

Choose a reason for hiding this comment

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

You cannot have the CVO base its decision about whether or not to allow a version change upon the operator status for the same reason you cannot allow it make a decision based on operand status.

who's setting this "upgradeable" condition if not the operator?

Copy link
Contributor Author

@deads2k deads2k Feb 11, 2019

Choose a reason for hiding this comment

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

You cannot have the CVO base its decision about whether or not to allow a version change upon the operator status for the same reason you cannot allow it make a decision based on operand status.

who's setting this "upgradeable" condition if not the operator?

Assuming you intend for Ready to mean your operator is functional. If an operator is Ready==false, then we should definitely allow upgrading that operator because otherwise it can be stuck broken. If an operator is Ready=true, then we should definitely allow upgrading that operator because it is functioning correctly. It's a condition unrelated to whether or not the CVO should allow an upgrade.

Copy link
Contributor

@soltysh soltysh Feb 12, 2019

Choose a reason for hiding this comment

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

How/when do you define that operator is not ready (Ready=false) but upgradable (Upgradable=true)? What's the algorithm/conditions behind that decision? Is it human made?

Copy link
Contributor Author

@deads2k deads2k Feb 12, 2019

Choose a reason for hiding this comment

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

How/when do you define that operator is not ready (Ready=false) but upgradable (Upgradable=true)? What's the algorithm/conditions behind that decision? Is it human made?

I think we're looking at different conditions and I'd like to have this pull focus on Upgradeable. It is an operator made choice.

)

// ClusterOperatorList is a list of OperatorStatus resources.
Expand Down