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

refact(upgrade): add control plane checks in pre-upgrade #8

Merged

Conversation

shubham14bajpai
Copy link
Contributor

Signed-off-by: shubham [email protected]

This PR adds pre-upgrade checks for control plane operators.

Copy link

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

provided comments

@@ -60,7 +61,11 @@ func NewCSPCPatch(opts ...CSPCPatchOptions) *CSPCPatch {

// PreUpgrade ...
func (obj *CSPCPatch) PreUpgrade() error {
err := obj.CSPC.PreChecks(obj.From, obj.To)
err := isOperatorUpgraded("cspc-operator", obj.Namespace, obj.To, obj.KubeClientset)

Choose a reason for hiding this comment

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

Should it go into precheck right(because it is mostly like check before upgrade)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Precheck is for a specific CR, it does not checks for any other resource. Preupgrade is for a volume or pool as a whole(means openebs CRs + deploy + service), so it makes more sense to have it in preupgrade.

Copy link

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

Changes are good

return fmt.Errorf("operator pod missing for %s", componentName)
}
for _, pod := range operatorPods.Items {
if pod.Labels["openebs.io/version"] != toVersion {

Choose a reason for hiding this comment

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

nit: We can move openebs.io/version to some global constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Will take this in separate PR.

Copy link

@prateekpandey14 prateekpandey14 left a comment

Choose a reason for hiding this comment

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

lgtm

@prateekpandey14 prateekpandey14 merged commit 5bc11fa into openebs-archive:master Jun 2, 2020
@shubham14bajpai shubham14bajpai deleted the control-plane branch August 19, 2020 05:46
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.

3 participants