Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

internal/olm/operator: fix operator-registry image for run packagemanifests #3856

Merged

Conversation

joelanford
Copy link
Member

Description of the change:

Fixed a bug with run packagemanifests that caused the underlying registry pod to fail to start. Changed the registry pod image from quay.io/openshift/origin-operator-registry:latest to quay.io/operator-framework/upstream-registry-builder:latest

Motivation for the change:
The run packagemanifests command is broken and needs to be fixed.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@joelanford joelanford changed the title internal/olm/operator: pin operator-registry image to 4.5 internal/olm/operator: change operator-registry image for run packagemanifests Sep 4, 2020
@joelanford joelanford force-pushed the fix-run-packagemanifests-e2e branch from bd7a825 to 1880840 Compare September 4, 2020 16:33
@joelanford joelanford changed the title internal/olm/operator: change operator-registry image for run packagemanifests internal/olm/operator: fix operator-registry image for run packagemanifests Sep 4, 2020
@camilamacedo86 camilamacedo86 added olm-integration Issue relates to the OLM integration kind/bug Categorizes issue or PR as related to a bug. labels Sep 4, 2020
)

const (
// The image operator-registry's initializer and registry-server binaries
// are run from.
// QUESTION(estroz): version registry image?
registryBaseImage = "quay.io/openshift/origin-operator-registry:latest"
registryBaseImage = "quay.io/operator-framework/upstream-registry-builder:latest"
Copy link
Contributor

Choose a reason for hiding this comment

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

I saw

 Waiting for Deployment \\\"default/e2e-fkkv-registry-server\\\" to rollout: 0 of 1 updated replicas are available\"\ntime=\"2020-09-04T15:34:20Z\" level=fatal msg=\"Failed to run packagemanifests: create catalog: error creating registry resources: error registering package: error waiting for Deployment \\\"default/e2e-fkkv-registry-server\\\" to roll out: timed out waiting for the condition\\n\"\n",

Copy link
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 nit
otherwise, /lgtm

@joelanford joelanford force-pushed the fix-run-packagemanifests-e2e branch from 1880840 to 859aef4 Compare September 4, 2020 17:41
Copy link
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.

/lgtm
the nit is not a blocker at all.

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

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

/lgtm

@camilamacedo86
Copy link
Contributor

@joelanford we have the test/integration using mock static data. It might be the case of the issue: https://travis-ci.org/github/operator-framework/operator-sdk/jobs/724251712#L1731

@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 4, 2020
@joelanford joelanford force-pushed the fix-run-packagemanifests-e2e branch 2 times, most recently from 98c39e1 to 958c0c9 Compare September 4, 2020 20:57
@joelanford joelanford force-pushed the fix-run-packagemanifests-e2e branch from 21c6141 to c34a635 Compare September 5, 2020 00:27
@joelanford joelanford force-pushed the fix-run-packagemanifests-e2e branch from c34a635 to 4f8cf1b Compare September 5, 2020 00:58
@Doout
Copy link

Doout commented Sep 8, 2020

@joelanford Have this been tested on OCP (Openshift)? When I try out your fork, I am hitting a panic when it tries to run /bin/initializer -o bundle.db

Error: unable to open database file
Usage:
   [flags]

Flags:
  -h, --help               help for this command
  -m, --manifests string   relative path to directory of manifests (default "manifests")
  -o, --output string      relative path to a sqlite file to create or overwrite (default "bundles.db")
      --permissive         allow registry load errors

panic: unable to open database file

When I debug the pod (oc debug) and found that I get permission denied with touch which I assume the SQL package will also hit this (unable to open database file coming from SQL package)

/registry $ touch bundle.db
touch: bundle.db: Permission denied

When I run it as root, I face no issues. I assume the permission for /registry folder is not correct when it get deploy on OCP.
On OCP, UID is random while /registry ownership is root and no write access for any user

drwxr-xr-x    3 root     root            23 Sep  8 13:12 registry

@jmrodri jmrodri merged commit 6c86606 into operator-framework:master Sep 8, 2020
@estroz
Copy link
Member

estroz commented Sep 8, 2020

@Doout this will need a follow-up to support all cluster types. Can you make an issue with the information in your comment above?

@joelanford joelanford deleted the fix-run-packagemanifests-e2e branch September 9, 2020 18:32
joelanford added a commit to joelanford/operator-sdk that referenced this pull request Sep 17, 2020
…ifests (operator-framework#3856)

* internal/olm/operator: fix operator-registry image for run packagemanifests

* internal/olm/client/client.go: return error for failed CSV

* use switch for DoCSVWait phase logic

* pin to olm 0.15.1 for integration tests
joelanford added a commit to joelanford/operator-sdk that referenced this pull request Sep 17, 2020
…ifests (operator-framework#3856)

* internal/olm/operator: fix operator-registry image for run packagemanifests

* internal/olm/client/client.go: return error for failed CSV

* use switch for DoCSVWait phase logic

* pin to olm 0.15.1 for integration tests
joelanford added a commit to joelanford/operator-sdk that referenced this pull request Sep 17, 2020
…ifests (operator-framework#3856)

* internal/olm/operator: fix operator-registry image for run packagemanifests

* internal/olm/client/client.go: return error for failed CSV

* use switch for DoCSVWait phase logic

* pin to olm 0.15.1 for integration tests
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. olm-integration Issue relates to the OLM integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants