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

ENTESB-14866: Improve logic of OLM Operator-Group installed for jaeger addon #9104

Merged
merged 2 commits into from Oct 7, 2020

Conversation

phantomjinx
Copy link
Contributor

@phantomjinx phantomjinx commented Oct 6, 2020

Please try out as part of review

  • The jaeger-operator should now end up being installed in the openshift-operators namespace (same as when installing through the console UI). This is intentional since a global-operators operator-group is already installed in that namespace so syndesis uses that rather than blindly creating its own.

  • It is also now possible to effectively uninstall the jaeger instance since its CR is owned by the syndesis-operator. Therefore, editing the syndesis CR to disable the jaeger addon will effectively delete the jaeger CR, which in turn causes the jaeger operator (installed in openshift-operators) to remove the syndesis-jaeger instance.

* install.go
 * Exposes a log-level switch when installing the operator
 * log-level -> 1 - 10

* operator.yml.tmpl
 * Marries the log-level parameter to the zap-level of the operator binary
   when run on the cluster. Zap log already built-in & configured to go
@phantomjinx phantomjinx self-assigned this Oct 6, 2020
@pull-request-size pull-request-size bot added the size/xl Extra large label Oct 6, 2020
@claudio4j
Copy link
Contributor

I tested on OCP 4.5.13, the jaeger-operator was installed on syndesis namespace

$ oc -n openshift-operators get all
No resources found in openshift-operators namespace.

$ oc -n syndesis get all -oname|grep jaeger
pod/jaeger-operator-74f695f499-klq5v
pod/syndesis-jaeger-6ddbd5fb58-zkwns
service/jaeger-operator-metrics
service/noauth-syndesis-jaeger-query
service/syndesis-jaeger-agent
service/syndesis-jaeger-collector
service/syndesis-jaeger-collector-headless
service/syndesis-jaeger-query
deployment.apps/jaeger-operator
deployment.apps/syndesis-jaeger
replicaset.apps/jaeger-operator-74f695f499
replicaset.apps/syndesis-jaeger-6ddbd5fb58
route.route.openshift.io/syndesis-jaeger

@claudio4j
Copy link
Contributor

claudio4j commented Oct 6, 2020

These are the steps to build and install

$ GOOSLIST=linux ./build.sh --operator-build go

$ ./dist/linux-amd64/syndesis-operator install cluster 
cluster resources were installed successfully

$ ./dist/linux-amd64/syndesis-operator grant -u admin
role syndesis-installer granted to admin

$ ./dist/linux-amd64/syndesis-operator install operator --log-level 5 --dev
operator was updated successfully

$ ./dist/linux-amd64/syndesis-operator install app --addons jaeger
syndesis application was installed successfully

operator log
https://gist.github.com/claudio4j/bcadbeadf7fc87a9e53af2ea615a5361

  • Ignore the consolelinks permission error, I changed action/install.go to just log the error instead to throw the error
if err := config.SetConsoleLink(ctx, rtClient, syndesis, syndesisRoute.Host()); err != nil {
    a.log.Info("error installing console link", "error", err)
}

I have not yet investigated why the current permissions are not working.

* The naive approach to installing an operator-group in the syndesis ns
  regardless of other operator-groups is not acceptable as it causes issues
  with the jaeger operator.

* If there is already an all-namespace operator-group installed in any
  namespace then this will impact the syndesis namespace. The
  openshift-operators namespace provides one out of the box. This is the
  designated location for all-namespace operators when installing through
  the openshift console.

* Ensures that the jaeger-operator's subscription ends up in the
  openshift-operators namespace to be consistent, without specifying it
  by name.

* subscription.go
 * Finds any installed operator-groups before falling back to creating one
   in the syndesis namespace. in the case of jaeger. latter should only
   occur if an all-ns operator-group is not available in the cluster.
@phantomjinx phantomjinx merged commit 2cb40f3 into syndesisio:master Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/xl Extra large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants