Skip to content

[run bundle] Use controller runtime client to create registry pod#3665

Merged
rashmigottipati merged 1 commit intooperator-framework:masterfrom
rashmigottipati:registry-pod-client
Aug 11, 2020
Merged

[run bundle] Use controller runtime client to create registry pod#3665
rashmigottipati merged 1 commit intooperator-framework:masterfrom
rashmigottipati:registry-pod-client

Conversation

@rashmigottipati
Copy link
Copy Markdown
Member

Description of the change:

  • Switch registry pod to use controller runtime client instead of kubernetes client-go

Motivation for the change:

  • Across run bundle, we are using controller runtime client, and to avoid creating another client to create the registry pod, switching to use controller runtime client for creation of the pod

Comment thread internal/olm/operator/internal/registry_pod.go Outdated
Comment thread internal/olm/operator/internal/registry_pod.go Outdated
Comment thread internal/olm/operator/internal/registry_pod.go Outdated
Comment thread internal/olm/operator/internal/registry_pod_test.go Outdated
Comment thread internal/olm/operator/internal/registry_pod_test.go Outdated
Comment thread internal/olm/operator/internal/registry_pod.go
Comment thread internal/olm/operator/internal/registry_pod.go Outdated
Comment thread internal/olm/operator/internal/registry_pod.go Outdated
Comment thread internal/olm/operator/internal/registry_pod.go Outdated
Comment thread internal/olm/operator/internal/registry_pod.go Outdated
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

just a few nits for me. Otherwise, the code changes show ok but need the look from who attend the design meetings as well.

@rashmigottipati rashmigottipati requested review from bharathi-tenneti, jmrodri and joelanford and removed request for shawn-hurley August 11, 2020 14:16
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

It shows ok for me 👍
/lgtm

however, please before merge gets +1 approval for who has been working on the same context and has been attending the meetings to discuss it.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2020
Copy link
Copy Markdown
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2020
Comment thread internal/olm/operator/internal/registry_pod.go Outdated
Copy link
Copy Markdown
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2020
@openshift-ci-robot
Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@rashmigottipati rashmigottipati merged commit 41d27c1 into operator-framework:master Aug 11, 2020
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.

5 participants