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

feat: Import Devworkspace Che Operator #925

Merged
merged 40 commits into from
Aug 11, 2021
Merged

Conversation

metlos
Copy link
Contributor

@metlos metlos commented Jul 13, 2021

What does this PR do?

This imports the Devworkspace Che Operator as a new controller inside che-operator.

The minimum amount of changes have been made to both codebases for the stuff to work.
In another words, the new controller pkg/controller/devworkspace is essentially still
completely standalone. It even still runs in its own reconcile loop even though it
reconciles CheCluster which is also reconciled by the main che controller.

The significant changes are are concentrated in cmd/manager/main.go, which sets the whole
show up.

In pkg/deploy/dev-workspace, we have a new approach for installing the devworkspace support.
The operator exits after a successfull installation, relying on Kubernetes to restart it.
When intialized again, it sees the CRDs for devworkspaces present in the cluster and
initializes the devworkspace reconciler.

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

eclipse-che/che#19408

How to test this PR?

  1. Install che-operator
  2. Remove the DWCO container for the che-operator deployment (not yet done in this PR)
  3. Change the che-operator image to quay.io/lkrejci/che-operator:issue-19408
  4. Enable devworkspaces in the CheCluster CR
  5. Notice the che-operator restart automatically after installation of Devworkspaces
  6. Try to create a devfile v2.
  7. Rejoice.

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@metlos metlos changed the title Import Devworkspace Che Operator feat: Import Devworkspace Che Operator Jul 15, 2021
@metlos metlos marked this pull request as ready for review July 22, 2021 15:00
@sparkoo
Copy link
Member

sparkoo commented Aug 7, 2021

/retest

@sparkoo
Copy link
Member

sparkoo commented Aug 7, 2021

/test v7-stable-to-nightly

1 similar comment
@sparkoo
Copy link
Member

sparkoo commented Aug 7, 2021

/test v7-stable-to-nightly

@tolusha
Copy link
Contributor

tolusha commented Aug 7, 2021

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/eclipse-che_che-operator/925/pull-ci-eclipse-che-che-operator-main-v7-stable-to-nightly/1423981878754414592/artifacts/stable-to-nightly/stable-to-nightly/artifacts/eclipse-che/che-operator-687f7b5599-dppmc/che-operator.log

E0807 14:02:57.890476       1 reflector.go:138] sigs.k8s.io/controller-runtime/pkg/cache/internal/informers_map.go:224: Failed to watch *v1alpha1.KubernetesImagePuller: failed to list *v1alpha1.KubernetesImagePuller: kubernetesimagepullers.che.eclipse.org is forbidden: User "system:serviceaccount:eclipse-che:che-operator" cannot list resource "kubernetesimagepullers" in API group "che.eclipse.org" at the cluster scope

@skabashnyuk
Copy link
Contributor

skabashnyuk commented Aug 7, 2021

@sparkoo
Copy link
Member

sparkoo commented Aug 9, 2021

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/eclipse-che_che-operator/925/pull-ci-eclipse-che-che-operator-main-v7-stable-to-nightly/1423981878754414592/artifacts/stable-to-nightly/stable-to-nightly/artifacts/eclipse-che/che-operator-687f7b5599-dppmc/che-operator.log

E0807 14:02:57.890476       1 reflector.go:138] sigs.k8s.io/controller-runtime/pkg/cache/internal/informers_map.go:224: Failed to watch *v1alpha1.KubernetesImagePuller: failed to list *v1alpha1.KubernetesImagePuller: kubernetesimagepullers.che.eclipse.org is forbidden: User "system:serviceaccount:eclipse-che:che-operator" cannot list resource "kubernetesimagepullers" in API group "che.eclipse.org" at the cluster scope

I've tested update from 7.34.0 into this PR version, and it works fine. However, I don't know details of the CI test deployment. I don't have imagepuller enabled (I guess, I don't know how to do it).
It's very hard to get anything from the CI job. I don't know where to find CheCluster, che-operator templates, che-operator image... I cannot find it in artifacts.

cc: @tolusha

@sparkoo sparkoo closed this Aug 9, 2021
@sparkoo sparkoo reopened this Aug 9, 2021
@sparkoo
Copy link
Member

sparkoo commented Aug 9, 2021

/retest

1 similar comment
@sparkoo
Copy link
Member

sparkoo commented Aug 9, 2021

/retest

@tolusha
Copy link
Contributor

tolusha commented Aug 9, 2021

I will take a look ci/prow/v7-stable-to-nightly

@tolusha
Copy link
Contributor

tolusha commented Aug 9, 2021

Here is a test script which is run on OpenShift CI.
So, it starts requiring cluster roles now [2]. I think a fix will be a pretty simple. But I would like to know why these changes are required now.

[1] https://github.com/eclipse-che/che-operator/blob/main/.ci/oci-nightly-update.sh
[2] https://github.com/eclipse-che/che-operator/blob/main/config/rbac/role.yaml#L121-L126

@skabashnyuk
Copy link
Contributor

@tolusha @metlos @sparkoo #925 (comment) all CQs has been approved.

@sparkoo sparkoo mentioned this pull request Aug 10, 2021
11 tasks
Copy link
Member

@sparkoo sparkoo left a comment

Choose a reason for hiding this comment

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

I won't be available until end of the week. I have no more comments to this issue except failing CI job, which I don't know how to properly fix. I have tested the PR and it is working fine for me. So I'm giving it a green.

@tolusha please feel free to merge when you're happy with the PR

Signed-off-by: Anatolii Bazko <[email protected]>
@openshift-ci
Copy link

openshift-ci bot commented Aug 11, 2021

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm label Aug 11, 2021
@openshift-ci
Copy link

openshift-ci bot commented Aug 11, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: metlos, sparkoo

The full list of commands accepted by this bot can be found 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

@tolusha
Copy link
Contributor

tolusha commented Aug 11, 2021

/retest

@eclipse-che eclipse-che deleted a comment from openshift-ci bot Aug 11, 2021
@tolusha
Copy link
Contributor

tolusha commented Aug 11, 2021

/test v8-multi-host-next-deployment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants