Operator status revisions #139

benjaminapetersen · 2019-02-14T20:37:00Z

Extracted a bunch of operator status updates out into separate named functions. Was a bit pedantic about naming, but hopefully it helps understand the flow and see if we can tease out any bugs.

@jhadvig

openshift-ci-robot · 2019-02-14T20:37:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benjaminapetersen

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [benjaminapetersen]

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

benjaminapetersen · 2019-02-14T20:37:51Z

pkg/console/operator/operator.go


-	// do we need the if(configChanged) update bits?
-	operatorConfigOut, consoleConfigOut, configChanged, err := sync_v400(c, operatorConfig, consoleConfig)
+	_, _, _, err := sync_v400(c, operatorConfig, consoleConfig)


I didn't notice this change, I need to look into it a bit.

benjaminapetersen · 2019-02-14T20:38:18Z

pkg/console/operator/status.go

@@ -0,0 +1,167 @@
+package operator


These are pretty verbose / repetitive.

benjaminapetersen · 2019-02-14T20:38:39Z

pkg/console/operator/sync_v400.go

+
 	toUpdate = toUpdate || depChanged

+	// at this point, we should not be failing anymore


Does this now make sense? Thinking through it.

it does, I'm counting on it as well

benjaminapetersen · 2019-02-14T20:40:07Z

Note that all of these ClusterOperator PRs will cause the Console operator to appear on the Cluster Settings page, under Cluster Operators:

benjaminapetersen · 2019-02-14T20:48:03Z

A test:
oc delete deployment console -n openshift-console
oc get events -n openshift-console-operator

shows:

LAST SEEN   TYPE      REASON                  KIND         MESSAGE
9m11s       Normal    Scheduled               Pod          Successfully assigned openshift-console-operator/console-operator-779cc894db-8b8vz to ip-10-0-133-68.us-west-1.compute.internal
9m3s        Normal    Pulling                 Pod          pulling image "quay.io/benjaminapetersen/console-operator:latest"
8m58s       Normal    Pulled                  Pod          Successfully pulled image "quay.io/benjaminapetersen/console-operator:latest"
8m58s       Normal    Created                 Pod          Created container
8m58s       Normal    Started                 Pod          Started container
9m11s       Normal    Killing                 Pod          Killing container with id cri-o://console-operator:Need to kill Pod
9m11s       Normal    SuccessfulCreate        ReplicaSet   Created pod: console-operator-779cc894db-8b8vz
8m39s       Normal    LeaderElection          ConfigMap    d4803d32-3097-11e9-a364-0a580a81020b became leader
8m39s       Warning   StatusNotFound          Deployment   Unable to determine current operator status for console
8m39s       Normal    OperatorStatusChanged   Deployment   Status for operator console changed: Failing set to False (""),Available set to True (""),Progressing set to True (""),status.relatedObjects changed from [] to [{"operator.openshift.io" "consoles" "" "console"} {"config.openshift.io" "consoles" "" "console"} {"oauth.openshift.io" "oauthclients" "" "console"} {"" "namespaces" "" "openshift-console-operator"} {"" "namespaces" "" "openshift-console"}]
8m39s       Normal    OperatorStatusChanged   Deployment   Status for operator console changed: Progressing changed from True to False (""),status.relatedObjects changed from [] to [{"operator.openshift.io" "consoles" "" "console"} {"config.openshift.io" "consoles" "" "console"} {"oauth.openshift.io" "oauthclients" "" "console"} {"" "namespaces" "" "openshift-console-operator"} {"" "namespaces" "" "openshift-console"}]
34s         Normal    DeploymentCreated       Deployment   Created Deploymentapps./console -n openshift-console because it was missing
34s         Normal    OperatorStatusChanged   Deployment   Status for operator console changed: Progressing changed from False to True ("")
34s         Normal    OperatorStatusChanged   Deployment   Status for operator console changed: Progressing changed from True to False ("")
34s         Normal    OperatorStatusChanged   Deployment   Status for operator console changed: Available changed from True to False ("NoDeploymentPodsAvailableOnAnyNode.")
32s         Normal    OperatorStatusChanged   Deployment   Status for operator console changed: Available changed from False to True ("")

Which is interesting, and
oc describe clusteroperator console
Will result in:

Name:         console
Namespace:
Labels:       <none>
Annotations:  <none>
API Version:  config.openshift.io/v1
Kind:         ClusterOperator
Metadata:
  Creation Timestamp:  2019-02-14T20:34:04Z
  Generation:          1
  Resource Version:    113010
  Self Link:           /apis/config.openshift.io/v1/clusteroperators/console
  UID:                 de84e7e6-3097-11e9-968a-02a640b76a0a
Spec:
Status:
  Conditions:
    Last Transition Time:  2019-02-14T20:42:24Z
    Status:                True
    Type:                  Available
    Last Transition Time:  2019-02-14T20:42:09Z
    Status:                False
    Type:                  Progressing
    Last Transition Time:  2019-02-14T20:34:04Z
    Status:                False
    Type:                  Failing
  Extension:               <nil>
  Related Objects:
    Group:     operator.openshift.io
    Name:      console
    Resource:  consoles
    Group:     config.openshift.io
    Name:      console
    Resource:  consoles
    Group:     oauth.openshift.io
    Name:      console
    Resource:  oauthclients
    Group:
    Name:      openshift-console-operator
    Resource:  namespaces
    Group:
    Name:      openshift-console
    Resource:  namespaces
  Versions:    <nil>
Events:        <none>

Which seems promising.

jhadvig · 2019-02-14T21:46:14Z

/retest

benjaminapetersen · 2019-02-14T22:11:50Z

/retest

benjaminapetersen · 2019-02-15T03:19:34Z

Fail message:

level=warning msg="Found override for ReleaseImage. Please be warned, this is not advised"
level=info msg="Consuming \"Install Config\" from target directory"
level=info msg="Creating cluster..."
level=info msg="Waiting up to 30m0s for the Kubernetes API..."
level=info msg="API v1.12.4+bb5f71c up"
level=info msg="Waiting up to 30m0s for the bootstrap-complete event..."
level=warning msg="RetryWatcher - getting event failed! Re-creating the watcher. Last RV: 4768"
level=info msg="Destroying the bootstrap resources..."
level=info msg="Waiting up to 30m0s for the cluster to initialize..."
level=fatal msg="failed to initialize the cluster: Cluster operator console has not yet reported success"

benjaminapetersen · 2019-02-15T03:19:43Z

/retest

jhadvig · 2019-02-15T09:00:54Z

pkg/console/operator/sync_v400.go

+
 	toUpdate = toUpdate || depChanged

+	// at this point, we should not be failing anymore


it does, I'm counting on it as well

jhadvig · 2019-02-15T09:09:39Z

pkg/console/operator/sync_v400.go

+
+	// but we may be in a transitional state, if any of the above resources changed
+	if toUpdate {
+		co.operatorStatusProgressing(operatorConfig)


was debugging a bit more and figured our that even if none of the resources are updated, here the toUpdate will be set to true, which shouldn't happen if nothing gets updated, right ?

Nope, if nothing is updated, we want it false. So that is definitely something worth tracking down.

benjaminapetersen · 2019-02-15T14:46:21Z

Just pasting again....

So this error:

level=fatal msg="failed to initialize the cluster: Cluster operator console has not yet reported success"

https://github.com/openshift/cluster-version-operator/blob/9335e43c21efc9df7eed48f05585997087fc04db/pkg/cvo/internal/operatorstatus.go#L126

Means we need Available: true, Progressing: false, Failing: false in order for CVO to be happy & tell the installer its happy.

benjaminapetersen · 2019-02-15T20:38:59Z

/retest

the brand fix PR went in.

benjaminapetersen · 2019-02-15T21:12:35Z

Prefer #142 for tidiness (squashed verison of this)

openshift-ci-robot · 2019-02-15T22:05:42Z

@benjaminapetersen: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws	`a7cca40`	link	`/test e2e-aws`
ci/prow/e2e-aws-operator	`a7cca40`	link	`/test e2e-aws-operator`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

benjaminapetersen · 2019-02-21T16:48:29Z

Closing, #142 has a few other necessary components for operator status, such as renumbering manifests, etc.

jhadvig and others added 8 commits February 14, 2019 09:56

Operator status

62e1e49

remove race

5939692

remove aggregation

29d48f5

1

c98bd0a

debug

5e1f548

Extract setUnmanagedConditions() into separate file/func

b8e4e18

f

b3a496e

Extract operator status updates into separate functions

9988728

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2019

openshift-ci-robot requested review from enj and spadgett February 14, 2019 20:37

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 14, 2019

benjaminapetersen commented Feb 14, 2019

View reviewed changes

pkg/console/operator/status.go

@@ -0,0 +1,167 @@

package operator

Copy link

Contributor Author

benjaminapetersen Feb 14, 2019

Choose a reason for hiding this comment

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

These are pretty verbose / repetitive.

benjaminapetersen commented Feb 14, 2019

View reviewed changes

f

657ac0f

Add log messages to operator status funcs

817efcc

jhadvig reviewed Feb 15, 2019

View reviewed changes

Revise all status.condition funcs to ensure symmetry

a7cca40

benjaminapetersen mentioned this pull request Feb 15, 2019

Operator status revisions squash #142

Merged

benjaminapetersen closed this Feb 21, 2019

benjaminapetersen deleted the operator-status-revisions branch February 21, 2019 16:49


		toUpdate = toUpdate \|\| depChanged

		// at this point, we should not be failing anymore

Operator status revisions #139

Operator status revisions #139

Uh oh!

Conversation

benjaminapetersen commented Feb 14, 2019

Uh oh!

openshift-ci-robot commented Feb 14, 2019

Uh oh!

benjaminapetersen Feb 14, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Feb 14, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Feb 14, 2019

Choose a reason for hiding this comment

Uh oh!

jhadvig Feb 15, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen commented Feb 14, 2019

Uh oh!

benjaminapetersen commented Feb 14, 2019

Uh oh!

jhadvig commented Feb 14, 2019

Uh oh!

benjaminapetersen commented Feb 14, 2019

Uh oh!

benjaminapetersen commented Feb 15, 2019

Uh oh!

benjaminapetersen commented Feb 15, 2019

Uh oh!

jhadvig Feb 15, 2019

Choose a reason for hiding this comment

Uh oh!

jhadvig Feb 15, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Feb 15, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen commented Feb 15, 2019

Uh oh!

benjaminapetersen commented Feb 15, 2019

Uh oh!

benjaminapetersen commented Feb 15, 2019

Uh oh!

openshift-ci-robot commented Feb 15, 2019

Uh oh!

benjaminapetersen commented Feb 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

benjaminapetersen commented Feb 21, 2019 •

edited

Loading