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

Bug 1694173: Improve status reporting #105

Merged

Conversation

@sallyom
Copy link
Contributor

sallyom commented Apr 9, 2019

Currently we always set Available=False when we set Progressing=True and Available=True/Progressing=False. This PR fine-tunes those to set independently when appropriate.

@openshift-ci-robot openshift-ci-robot requested review from mrogers950 and stlaz Apr 9, 2019
@sallyom sallyom changed the title Bug 1694173: Improve status reporting Bug 1694173: Improve status reporting [WIP] Apr 9, 2019
@sallyom sallyom force-pushed the sallyom:improve-status-reporting branch from b777110 to 05d5b88 Apr 9, 2019
pkg/operator2/operator.go Outdated Show resolved Hide resolved
@sallyom sallyom force-pushed the sallyom:improve-status-reporting branch from 05d5b88 to 9dd4af5 Apr 9, 2019
@openshift-ci-robot openshift-ci-robot added size/M and removed size/L labels Apr 9, 2019
@sallyom sallyom force-pushed the sallyom:improve-status-reporting branch from 9dd4af5 to b4dd220 Apr 9, 2019
@sallyom

This comment has been minimized.

Copy link
Contributor Author

sallyom commented Apr 9, 2019

/retest

@sallyom

This comment has been minimized.

Copy link
Contributor Author

sallyom commented Apr 9, 2019

@enj I tested an upgrade to this PR payload and guess what? authentication CO was not the very last CO to report new version & Available=True, first time I've seen that. Upgrade was successful.

@sallyom

This comment has been minimized.

Copy link
Contributor Author

sallyom commented Apr 10, 2019

/retest

}

if osinDeployment.Status.UpdatedReplicas != osinDeployment.Status.Replicas || osinDeployment.Status.UnavailableReplicas > 0 {
return false, "not all deployment replicas are ready", nil
setProgressingTrue(operatorConfig, reason, "not all deployment replicas are ready")

This comment has been minimized.

Copy link
@enj

enj Apr 11, 2019

Contributor

We are available here as long as we have one ready replica.

This comment has been minimized.

Copy link
@sallyom

sallyom Apr 11, 2019

Author Contributor

right, updated, missed that!

@@ -376,7 +380,8 @@ func (c *authOperator) handleSync(operatorConfig *operatorv1.Authentication) err
if c.versionGetter.GetVersions()[operatorSelfName] != operatorVersion {
c.versionGetter.SetVersion(operatorSelfName, operatorVersion)
}
c.setAvailableStatus(operatorConfig)
setAvailableTrue(operatorConfig, "ManagedResourcesSuccessfullyApplied")

This comment has been minimized.

Copy link
@enj

enj Apr 11, 2019

Contributor

AsExpected?

This comment has been minimized.

Copy link
@sallyom

sallyom Apr 11, 2019

Author Contributor

that works

@sallyom sallyom force-pushed the sallyom:improve-status-reporting branch from b4dd220 to a6e01f8 Apr 11, 2019
@enj

This comment has been minimized.

Copy link
Contributor

enj commented Apr 11, 2019

/lgtm

@enj
enj approved these changes Apr 11, 2019
@openshift-ci-robot

This comment has been minimized.

Copy link

openshift-ci-robot commented Apr 11, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enj, sallyom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot

This comment has been minimized.

Copy link

openshift-ci-robot commented Apr 11, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enj, sallyom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@enj enj changed the title Bug 1694173: Improve status reporting [WIP] Bug 1694173: Improve status reporting Apr 11, 2019
@openshift-merge-robot openshift-merge-robot merged commit 2928126 into openshift:master Apr 11, 2019
4 of 5 checks passed
4 of 5 checks passed
tide Not mergeable.
Details
ci/prow/e2e-aws Job succeeded.
Details
ci/prow/e2e-aws-operator Job succeeded.
Details
ci/prow/images Job succeeded.
Details
ci/prow/unit Job succeeded.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.