Skip to content
This repository has been archived by the owner on Jul 30, 2021. It is now read-only.

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

Merged

Conversation

davidfestal
Copy link
Contributor

@davidfestal davidfestal commented May 22, 2019

This PR is a followup of PR #280 that only focuses on the community-operators addition, as requested by comment #280 (comment)

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

Tested on 4.0.0-0.9

image

image

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 22, 2019
@davidfestal davidfestal changed the title Add community Eclipse Che operator (for downstream operators) [WIP] Add community Eclipse Che operator (for downstream operators) May 23, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 23, 2019
@davidfestal
Copy link
Contributor Author

There was still one unanswered question in the previous PR (superseded by this one): #280 (comment)

To summarize:

One of the possible settings in a CheCluster CR, which is disabled by default is auth.openShiftoAuth to enable authenticating against the current Openshift OAuth server.
It requires additional cluster permissions.

As @dmesser suggested, there are 2 options here:

  • Either we consider this use-case as a niche, rather rare, use-case: then we would let the end-user add the permissions manually, as it is currently done in this PR (and documented in the CSV description)
  • Or we can consider this use-case as an important and rather standard one: then we would prefer adding these permissions directly in the CSV permissions section, even if these extended permissions are not required in any case.

@l0rd @slemeur @nickboldt wdyt ?

@davidfestal davidfestal changed the title [WIP] Add community Eclipse Che operator (for downstream operators) Add community Eclipse Che operator (for downstream operators) May 27, 2019
@openshift-ci-robot 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 27, 2019
@davidfestal davidfestal changed the title Add community Eclipse Che operator (for downstream operators) [WIP] Add community Eclipse Che operator (for downstream operators) May 27, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 27, 2019
@davidfestal davidfestal changed the title [WIP] Add community Eclipse Che operator (for downstream operators) Add community Eclipse Che operator (for downstream operators) May 28, 2019
@openshift-ci-robot 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 28, 2019
@l0rd
Copy link

l0rd commented May 28, 2019

@davidfestal for option 2 it's not clear what happens if the users doesn't change any config and try to deploy on plain k8s: is the operator going to fail or will it figure out that there is no OpenShift oAuth and adapt the deployment to the underlying infrastructure? If it fails I would rather go with option 1 because that's the community operator: most of the deployments will happen on plain k8s and the default configuration should reflect that (k8s first).

@davidfestal
Copy link
Contributor Author

@l0rd This PR is for Openshift only. The corresponding PR for plain K8s doesn't even have the explanation about the additional requirements, because it has no sense.
As for how the che-operator code behaves when it is installed on plain K8s and the user enabled Openshift OAuth in the CR, I just tested and it seems it is simply ignored.

@dmesser
Copy link
Collaborator

dmesser commented May 31, 2019

@davidfestal Can you please rebase and squash your commits?

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 1, 2019
@davidfestal davidfestal force-pushed the community-che-operator branch from c13f98e to 6f961d2 Compare June 4, 2019 12:38
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 4, 2019
* Add repository URL to annotations
* Upgrade to `beta-5.0`
* Rename the CSV to be OperatorHub-compliant

Signed-off-by: David Festal <[email protected]>
@davidfestal davidfestal force-pushed the community-che-operator branch from 6f961d2 to 50d008d Compare June 4, 2019 16:20
@dmesser dmesser merged commit 4cc0389 into operator-framework:master Jun 4, 2019
@openshift-ci-robot
Copy link
Collaborator

@davidfestal: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/scorecard 50d008d link /test scorecard

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.

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants