Skip to content

[run bundle]: Create bundle registry server#3337

Merged
rashmigottipati merged 1 commit intooperator-framework:masterfrom
rashmigottipati:create-registry-pod
Jul 15, 2020
Merged

[run bundle]: Create bundle registry server#3337
rashmigottipati merged 1 commit intooperator-framework:masterfrom
rashmigottipati:create-registry-pod

Conversation

@rashmigottipati
Copy link
Copy Markdown
Member

Description of the change:

https://issues.redhat.com/browse/OSDK-1304

In this PR,

  • Create a kubernetes clientset and using the client, create a registry pod using a registryImage and set the container command (entrypoint) to:
    {{/bin/bash -c ‘ \ /bin/mkdir -p /database && \ /bin/opm registry add -d /database/index.db -b {.BundleImage} && \ /bin/opm registry serve -d /database/index.db’}}
  • Before creation, verify if the registry pod already exists and return the existing pod if found
  • After creation of the pod, verify that it was successfully created by polling and verifying that pod status is running
  • Added functionality for capturing pod logs
  • Manual testing is done, unit testing is in progress

Note: Once the CLI code is ready, building the config and creating the client will be setup once in the CLI side, and passed along to this function.

Motivation for the change:

@rashmigottipati rashmigottipati added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 2, 2020
@rashmigottipati rashmigottipati self-assigned this Jul 2, 2020
@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 Jul 2, 2020
@rashmigottipati rashmigottipati added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 2, 2020
@rashmigottipati rashmigottipati requested a review from jmrodri July 2, 2020 17:31
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.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
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
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
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
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 Outdated
Comment thread internal/olm/operator/internal/registry_pod.go
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
Comment thread internal/olm/operator/internal/registry_pod.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
@rashmigottipati rashmigottipati removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2020
Comment thread internal/olm/operator/internal/registry_pod.go Outdated
@rashmigottipati rashmigottipati force-pushed the create-registry-pod branch 2 times, most recently from 3a0cd37 to 131f5c3 Compare July 8, 2020 18:49
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.

Assuming that it should not be called in any sdk command yet and that has a consensus to do ginkgo/gomega in a follow-up

It shows /lgtm /approve for me.

@rashmigottipati rashmigottipati force-pushed the create-registry-pod branch 2 times, most recently from 6926ffb to 5a4e465 Compare July 14, 2020 14:17
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.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
@joelanford joelanford self-requested a review July 14, 2020 18:52
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.

One nit then

/lgtm

Comment thread internal/olm/operator/internal/registry_pod.go Outdated
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2020
Comment thread go.mod Outdated
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2020
@rashmigottipati rashmigottipati force-pushed the create-registry-pod branch 3 times, most recently from bd38e07 to 5d6b071 Compare July 14, 2020 21:47
@jmrodri
Copy link
Copy Markdown
Member

jmrodri commented Jul 14, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2020
- Validate RegistryPod struct fields and set defaults

- Verify pod creation using polling and ensuring pod status is Running

- Add logic to check if pod already exists and return existing pod

- Add *corev1.Pod as an unexported field to RegistryPod struct

- Make kubernetes client exported on RegistryPod so the caller can set it

- Add templating for container command

- Add logic to wrap pod logs inside error in case of failures

- Use k8sutil's TrimDNS1123Label func to construct pod name

- Add unit tests in ginkgo/gomega
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2020
@jmrodri
Copy link
Copy Markdown
Member

jmrodri commented Jul 14, 2020

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2020
@jmrodri jmrodri dismissed joelanford’s stale review July 15, 2020 00:44

Joe's comments were addressed by Rashmi.

@rashmigottipati rashmigottipati merged commit e4f0d86 into operator-framework:master Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants