Add Eclipse Che operator #280

ghost · 2019-04-13T12:03:32Z

This PR adds Eclipse Che operator to community operators and upstream community operators. Eclipse Che operator auto detects infra, so can run on vanilla k8s and OpenShift.

Please let me know if I have chosen the wrong categories.

Tested on 4.0.0-0.9

openshift-ci-robot · 2019-04-13T12:03:54Z

Hi @eivantsov. Thanks for your PR.

I'm waiting for a operator-framework or openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

ghost · 2019-04-13T12:21:47Z

Can someone help with the failing build please?

ERROR:operatorcourier.validate:category Developer Tools is not a valid category

It looks like it was allowed to provide own categories. It looks like it's not the case now. Is there a list of valid categories I can choose from?

Btw, I have upgraded operator-courier to the latest version, and verify does not report anything illegal.

gallettilance · 2019-04-15T14:34:35Z

Here is a list of the current allowed categories https://github.com/operator-framework/community-operators/blob/master/docs/required-fields.md#categories

ghost · 2019-04-15T14:36:07Z

@galletti94 thanks. What would be the right way to proceed if none of those fits Eclipse Che operator? Is it possible to contribute a new category?

gallettilance · 2019-04-15T14:41:21Z

Good question. I will escalate this concern and get back to you on that.

Short term - removing that field, will categorize your operator as "other" - if this is acceptable for you in the short term it will help accelerate getting your operator into operatorhub.io. We can always add the category in a later PR (once this category becomes available).

ghost · 2019-04-15T14:42:54Z

@galletti94 got it. By the way, it looks like this is a new requirement since when I issued #193 build was a success.

Will this new requirement affect #193 too?

gallettilance · 2019-04-15T14:54:39Z

The UI validation done in this PR is one currently only done for operatorhub.io PRs so #193 will not be affected. However we do plan on using that same validation for both OCP and operatorhub.io in the near future - possibly as early as next week.

gallettilance · 2019-04-16T12:55:46Z

@eivantsov To add a category to the list of accepted categories, please open a PR in this repo to add that category to the docs. In the PR, please briefly describe why this additional category is needed.

Once it is merged I will add that category to operator-courier. Note that this change will only be visible when a new release of courier is cut (which may take a few days).

SamiSousa · 2019-04-18T15:18:49Z

upstream-community-operators/eclipse-che/eclipse-che.7.0.0-beta-3.0.csv.yaml

+              verbs:
+                - "*"
+            - apiGroups:
+                - route.openshift.io


If this operator has a dependency on this openshift API, would this still work on vanilla kubernetes?

@SamiSousa Yes, when on k8s infra, the operator will create ingresses rather than routes. In csv, I request permissions to manage both

When running this operator on minikube, the operator logs repeatedly print this message:

E0426 17:39:16.986396 1 reflector.go:134] sigs.k8s.io/controller-runtime/pkg/cache/internal/informers_map.go:126: Failed to list *v1beta1.Ingress: ingresses.extensions is forbidden: User "system:serviceaccount:eclipse-che:che-operator" cannot list resource "ingresses" in API group "extensions" in the namespace "eclipse-che"

After deploying the example CR, it's clear to me that the operator is unable to resolve it.

@SamiSousa thanks for checking that. Is it possible that the APIGroup is wrong? For example, in Jaeger operator it's extensions, while it's extensions/v1beta1 for Eclipse Che. Weird thing though is that I cannot reproduce it on minikube myself, though I haven't ever tested with OLM/Marketplace on kube (only OCP/OKD).

Anyways, I have fixed API group for ingresses. I'd appreciate if you share instructions on how you test on minikube. Thanks.

I've tested this again and still have this error:

ce "eclipse-che" E0430 16:48:42.334461 1 reflector.go:134] sigs.k8s.io/controller-runtime/pkg/cache/internal/informers_map.go:126: Failed to list *v1beta1.Ingress: ingresses.extensions is forbidden: User "system:serviceaccount:eclipse-che:che-operator" cannot list resource "ingresses" in API group "extensions" in the namespace "eclipse-che"

@SamiSousa are ingresses in the role that is bound to che-operator SA?

Also, can you please share steps you take to test it? On MiniKube I am applying yamls with role, rolebinding and service account manually (I never tried OLM and Marketplace on k8s)

Checked it again on MiniKube. This is the role https://github.com/eclipse/che-operator/blob/master/deploy/role.yaml, and it worked, service account can manage ingresses.

@SamiSousa what is your MiniKube version? It is related to RBAC but I cannot figure out why it fails on your side.

minikube version: v0.33.1

gallettilance · 2019-04-18T17:20:06Z

@eivantsov @SamiSousa FYI I just added a PR to operator-courier to add this category.

bmicklea · 2019-04-29T15:20:55Z

Now that the developer tools category exists is this in a merge-able state?

ghost · 2019-04-29T15:23:52Z

@bmicklea there are still some unresolved comments. I have pushed a fix, waiting for @SamiSousa to confirm.

ssimk0 · 2019-04-30T09:36:08Z

upstream-community-operators/eclipse-che/eclipse-che.7.0.0-beta-3.0.csv.yaml

+    To instruct the operator to skip deploying PostgreSQL and Keycloak and connect to an existing DB and Keycloak instead:
+
+    * set respective fields to `true` in a custom resource spec
+    * provide the operator with connection and authentication details:


Hi,
here you probably miss the empty line, because it's not render as second item but as:

set respective fields to true in a custom resource spec * provide the operator with connection and authentication details:

you can test it here and also it will be look probably little better when you should tab little the code section above them

@ssimk0 thank you. Fixed

dmesser · 2019-05-02T09:02:49Z

@eivantsov - the description talks about pre-requisites for OpenShift. Given that you also submit to OperatorHub.io you also target the wider Kubernetes community. Are there any pre-requisites there too?
Otherwise /lgtm

ghost · 2019-05-02T09:08:07Z

@dmesser no, there are no pre reqs for Kubernetes, default role+ role binding is sufficient on k8s.

dmesser · 2019-05-02T09:43:43Z

Ok cool. Since you submit copies of your CSV to both community (OpenShift) and upstream-community (Kubernetes), you could omit the OpenShift specific stuff in the Kubernetes-focussed version of your CSV.

ghost · 2019-05-02T10:25:06Z

@dmesser thanks. Fixed

dmesser · 2019-05-02T15:39:23Z

@eivantsov great, thanks! Last nit: the one-liner description still mentions OpenShift - this could confuse users.

Also, can you make the code example in the description in triple backticks wrapping a block instead of single backticks per individual line? Otherwise /lgtm

ghost · 2019-05-02T17:17:28Z

@dmesser thanks for your review. Fixed.

SamiSousa · 2019-05-03T18:55:55Z

@eivantsov Looks good! Could you squash your commits please? 🙂

SamiSousa · 2019-05-03T19:49:14Z

community-operators/eclipse-che/eclipse-che.7.0.0-beta-3.0.csv.yaml

+
+    oc create clusterrole che-operator --resource=oauthclients --verb=get,create,delete,update,list,watch
+
+    oc create clusterrolebinding che-operator --clusterrole=che-operator --serviceaccount=${NAMESPACE}:che-operator


Why can't this additional clusterrole be specified in the CSV's clusterPermissions section?

@SamiSousa

If I understand correctly, this might be to avoid adding unnecessary permissions, since these permissions would only be required when auth.openShiftoAuth is enabled, which is not by default.
Do you think these permissions should be added systematically ?

and wouldn't it represent some unnecessary security risk ?

dmesser · 2019-05-16T21:36:51Z

Can you separate the submission to community-operators and upstream-community-operators into a separate PR? This way we can validaty faster and accordingly to the target platform (OCP vs. vanilla k8s)

bmicklea · 2019-05-17T16:09:32Z

@davidfestal can you help out with this?

davidfestal · 2019-05-17T16:12:02Z

@bmicklea Sure, I'll look into it next week

davidfestal · 2019-05-22T16:26:47Z

Can you separate the submission to community-operators and upstream-community-operators into a separate PR? This way we can validaty faster and accordingly to the target platform (OCP vs. vanilla k8s)

@dmesser The 2 new split new PRs are #383 and #384

However I flagged them as WIP because I still want to update them with the last 7.0.0-beta-5.0
Che release.

Add Eclipse Che operator

6cd11e8

openshift-ci-robot requested review from SamiSousa and tkashem April 13, 2019 12:03

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 13, 2019

Fix build

27b6717

Eugene Ivantsov added 2 commits April 13, 2019 15:26

Add repository URL to annotations

e8a1cec

Trying to fix a build

c118190

ghost mentioned this pull request Apr 18, 2019

Add Developer Tools category #290

Merged

SamiSousa reviewed Apr 18, 2019

View reviewed changes

l0rd mentioned this pull request Apr 23, 2019

Che 7 GA eclipse-che/che#12696

Closed

SamiSousa mentioned this pull request Apr 26, 2019

[scripts] Update semver regex #309

Merged

Fix ingress API group

8f87661

ssimk0 reviewed Apr 30, 2019

View reviewed changes

Fix formatting

a019cec

Remove OpenShift related instructions

89d6083

Update csvs

98f0936

SamiSousa reviewed May 3, 2019

View reviewed changes

l0rd mentioned this pull request May 13, 2019

Single-user deploy on OCP 4 fails to create workspaces eclipse-che/che#13328

Closed

ibuziuk mentioned this pull request May 22, 2019

Getting the community Che Operator PR merged redhat-developer/rh-che#1422

Closed

This was referenced May 22, 2019

Add community Eclipse Che operator (for upstream operators) #383

Merged

Add community Eclipse Che operator (for downstream operators) #384

Merged

dmesser closed this May 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Eclipse Che operator #280

Add Eclipse Che operator #280

ghost commented Apr 13, 2019

openshift-ci-robot commented Apr 13, 2019

ghost commented Apr 13, 2019

gallettilance commented Apr 15, 2019

ghost commented Apr 15, 2019

gallettilance commented Apr 15, 2019

ghost commented Apr 15, 2019

gallettilance commented Apr 15, 2019 •

edited

Loading

gallettilance commented Apr 16, 2019

SamiSousa Apr 18, 2019

ghost Apr 18, 2019

SamiSousa Apr 26, 2019

ghost Apr 29, 2019

SamiSousa Apr 30, 2019

ghost Apr 30, 2019 •

edited by ghost

Loading

ghost Apr 30, 2019 •

edited by ghost

Loading

gallettilance commented Apr 18, 2019

bmicklea commented Apr 29, 2019

ghost commented Apr 29, 2019

ssimk0 Apr 30, 2019

ghost Apr 30, 2019

dmesser commented May 2, 2019

ghost commented May 2, 2019

dmesser commented May 2, 2019

ghost commented May 2, 2019

dmesser commented May 2, 2019

ghost commented May 2, 2019

SamiSousa commented May 3, 2019 •

edited

Loading

SamiSousa May 3, 2019

davidfestal May 22, 2019

davidfestal May 22, 2019 •

edited

Loading

dmesser commented May 16, 2019

bmicklea commented May 17, 2019

davidfestal commented May 17, 2019

davidfestal commented May 22, 2019


		oc create clusterrole che-operator --resource=oauthclients --verb=get,create,delete,update,list,watch

		oc create clusterrolebinding che-operator --clusterrole=che-operator --serviceaccount=${NAMESPACE}:che-operator

Add Eclipse Che operator #280

Add Eclipse Che operator #280

Conversation

ghost commented Apr 13, 2019

openshift-ci-robot commented Apr 13, 2019

ghost commented Apr 13, 2019

gallettilance commented Apr 15, 2019

ghost commented Apr 15, 2019

gallettilance commented Apr 15, 2019

ghost commented Apr 15, 2019

gallettilance commented Apr 15, 2019 • edited Loading

gallettilance commented Apr 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Apr 30, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

ghost Apr 30, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

gallettilance commented Apr 18, 2019

bmicklea commented Apr 29, 2019

ghost commented Apr 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmesser commented May 2, 2019

ghost commented May 2, 2019

dmesser commented May 2, 2019

ghost commented May 2, 2019

dmesser commented May 2, 2019

ghost commented May 2, 2019

SamiSousa commented May 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfestal May 22, 2019 • edited Loading

Choose a reason for hiding this comment

dmesser commented May 16, 2019

bmicklea commented May 17, 2019

davidfestal commented May 17, 2019

davidfestal commented May 22, 2019

gallettilance commented Apr 15, 2019 •

edited

Loading

ghost Apr 30, 2019 •

edited by ghost

Loading

ghost Apr 30, 2019 •

edited by ghost

Loading

SamiSousa commented May 3, 2019 •

edited

Loading

davidfestal May 22, 2019 •

edited

Loading