add logic for openshift webconsole operator #19498

deads2k · 2018-04-24T22:40:28Z

Adds a simple operator that creates a webconsole deployment. It manages conflicts with the workload controller by actively stomping it when the generation doesn't match. Errors appear on the status of the subresource

openshift-ci-robot · 2018-04-24T22:40:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k

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~~ [deads2k]

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

mfojtik · 2018-04-25T08:25:47Z

pkg/cmd/openshift-operators/util/resourceapply/apps.go

+	}
+	existing.Spec = *required.Spec.DeepCopy()
+
+	actual, err := client.Deployments(required.Namespace).Update(existing)


where do we handle conflicts? does the controller just re-apply on next sync?

where do we handle conflicts? does the controller just re-apply on next sync?

Yes, and it is logged and put into the CRD status.

mfojtik · 2018-04-25T10:18:39Z

pkg/cmd/openshift-operators/util/resourceread/core.go

+}
+
+func ReadConfigMapOrDie(objBytes []byte) *corev1.ConfigMap {
+	requiredObj, err := runtime.Decode(coreCodecs.UniversalDecoder(corev1.SchemeGroupVersion), []byte(objBytes))


[]byte(objByte) is a noop here right?

mfojtik · 2018-04-25T10:19:53Z

pkg/cmd/openshift-operators/util/resourceread/core.go

+	if err != nil {
+		panic(err)
+	}
+	return requiredObj.(*corev1.ConfigMap)


can we also panic when requiredObj != corev1.ConfigMap ?

can we also panic when requiredObj != corev1.ConfigMap ?

Yeah. it's a coding error.

mfojtik · 2018-04-25T10:31:47Z

pkg/cmd/openshift-operators/webconsole-operator/v310_00/deployment.go

+    webconsole: "true"
+spec:
+  strategy:
+    type: Recreate


is there reason this is Recreate and not Rollout ?

It was discussed some time ago, web-console switched to recreate some time ago, iirc.

deads2k · 2018-05-02T21:50:47Z

@mfojtik @jpeeler any known problems with service catalog? Doesn't immediately look related to my changes.

jpeeler · 2018-05-03T10:41:19Z

@deads2k There is, this PR needs to be merged: #19568. I really expected for it to land overnight. ..
Btw, what link are you looking at for failing catalog tests? Reporting has been disabled temporarily until the above PR merges.

deads2k · 2018-05-03T11:50:46Z

@deads2k There is, this PR needs to be merged: #19568. I really expected for it to land overnight. ..
Btw, what link are you looking at for failing catalog tests? Reporting has been disabled temporarily until the above PR merges.

Cluster up tests when you modify cluster up indicate it here: https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/19498/test_pull_request_origin_extended_clusterup/1021/

deads2k · 2018-05-05T12:36:49Z

/retest

jpeeler · 2018-05-07T15:27:36Z

/retest
I saw the same error with catalog, then pulled/updated everything and now it seems to have gone away.

jpeeler · 2018-05-07T16:44:38Z

I can't figure out why catalog is failing to start in the extended clusterup test. Other PRs seem to be successfully passing.

mfojtik · 2018-05-09T11:06:26Z

/retest

mfojtik · 2018-05-09T11:09:22Z

pkg/oc/bootstrap/clusteradd/components/web-console-operator/web_console_operator.go

-		// TODO wait until the webconsole is up
+		// wait until the webconsole to an available endpoint
+		WaitCondition: func() (bool, error) {
+			glog.V(2).Infof("polling for service catalog api server endpoint availability")


polling for webconsole availability?

mfojtik · 2018-05-09T11:50:12Z

@deads2k I think this is failing because you use the Always as the OPENSHIFT_PULL_POLICY which means the hypershift does not have the experimental subcommand (at least that is the case for my locally when I will get the same timeout...). Will make a PR against your PR.

deads2k · 2018-05-09T20:16:37Z

ah, image layout changed

deads2k · 2018-05-09T22:15:48Z

/retest

mfojtik · 2018-05-10T12:16:34Z

/lgtm

this is a good starting point

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 24, 2018

openshift-ci-robot requested review from csrwng and liggitt April 24, 2018 22:40

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 24, 2018

mfojtik reviewed Apr 25, 2018

View reviewed changes

deads2k changed the title ~~[WIP] add logic for openshift webconsole operator~~ add logic for openshift webconsole operator May 2, 2018

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 2, 2018

deads2k force-pushed the op-05-no-merge branch from c83ee69 to ce16292 Compare May 2, 2018 19:58

deads2k mentioned this pull request May 2, 2018

UPSTREAM: <carry>: enable CRD subresources #19598

Closed

deads2k force-pushed the op-05-no-merge branch from ce16292 to 681f9d2 Compare May 2, 2018 20:13

mfojtik reviewed May 9, 2018

View reviewed changes

deads2k added 2 commits May 9, 2018 15:31

add logic for openshift webconsole operator

ac52ff5

use webconsole operator in oc cluster up

948705f

deads2k force-pushed the op-05-no-merge branch from 681f9d2 to 457d321 Compare May 9, 2018 19:34

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 9, 2018

up: add pull policy to context and pass it properly to cluster add

9941843

deads2k force-pushed the op-05-no-merge branch from 457d321 to 9941843 Compare May 9, 2018 20:16

openshift-ci-robot assigned mfojtik May 10, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 10, 2018

openshift-merge-robot merged commit 9fd7944 into openshift:master May 10, 2018

deads2k deleted the op-05-no-merge branch July 3, 2018 17:48

add logic for openshift webconsole operator #19498

add logic for openshift webconsole operator #19498

Uh oh!

Conversation

deads2k commented Apr 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deads2k commented May 2, 2018

Uh oh!

jpeeler commented May 3, 2018

Uh oh!

deads2k commented May 3, 2018

Uh oh!

deads2k commented May 5, 2018

Uh oh!

jpeeler commented May 7, 2018

Uh oh!

jpeeler commented May 7, 2018

Uh oh!

mfojtik commented May 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfojtik commented May 9, 2018

Uh oh!

deads2k commented May 9, 2018

Uh oh!

deads2k commented May 9, 2018

Uh oh!

mfojtik commented May 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

deads2k commented Apr 24, 2018 •

edited

Loading