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.

Sign up for GitHub

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

Jump to bottom

refactor(images): use operator-framework namespace for all olm images #752

Merged
merged 6 commits into from
Mar 15, 2019
Merged

refactor(images): use operator-framework namespace for all olm images #752

merged 6 commits into from
Mar 15, 2019

Conversation

njhale
Copy link
Member

@njhale njhale commented Mar 12, 2019

Add a few changes around upstream images and packaging:

  • Use OLM images for all OLM images
  • Use existing manifests in package_quickstart.sh
  • Add quickstart generation to make release rule
  • Build with go 1.12 in upstream.Dockerfile

Addresses part of #747 and ALM-937.

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 12, 2019
@njhale
Copy link
Member Author

njhale commented Mar 12, 2019

/retest

@njhale njhale force-pushed the change-upstream-base branch from da4ea6c to d7ab91b Compare March 12, 2019 13:32
ecordell
ecordell approved these changes Mar 12, 2019
View reviewed changes
Copy link
Member

@ecordell ecordell left a comment

Choose a reason for hiding this comment

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

/lgtm

scripts/build_local.sh
docker build -f e2e.Dockerfile .
docker tag $(docker images --filter 'label=stage=olm' --format '{{.CreatedAt}}\t{{.ID}}' | sort -nr | head -n 1 | cut -f2) quay.io/coreos/olm:local
docker tag $(docker images --filter 'label=stage=builder' --format '{{.CreatedAt}}\t{{.ID}}' | sort -nr | head -n 1 | cut -f2) quay.io/coreos/olm-e2e:local
docker build -f upstream.Dockerfile .
Copy link
Member

Choose a reason for hiding this comment

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

Why the switch to upstream here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it would be nice to use the same image for local testing - I can put it back if you'd like.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2019
@njhale
Copy link
Member Author

njhale commented Mar 12, 2019

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 12, 2019
@alecmerdler
Copy link
Member

Note that we are using an image in operatorframework in the console end-to-end tests for OLM (to add a CatalogSource without Marketplace). I pushed the same image to the operator-framework org, but haven't made a PR against openshift/console to use it yet. Hopefully no one deletes that organization.

@ecordell
Copy link
Member

@njhale looks like a rebase is in order

@njhale
Copy link
Member Author

njhale commented Mar 12, 2019

/hold cancel
/retest

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 12, 2019
@njhale njhale force-pushed the change-upstream-base branch from d7ab91b to 92290c7 Compare March 12, 2019 20:54
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2019
@njhale
Copy link
Member Author

njhale commented Mar 12, 2019

Hopefully no one deletes that organization.

@alecmerdler we're using those images in OLM's e2e tests as well. I was planning on opening a PR with the upstream.Dockerfile for operator-registry today and rigging the Quay build-triggers.

alecmerdler
alecmerdler approved these changes Mar 12, 2019
View reviewed changes
Copy link
Member

@alecmerdler alecmerdler 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 Mar 12, 2019
@njhale njhale force-pushed the change-upstream-base branch from 92290c7 to 04b4d30 Compare March 12, 2019 23:20
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2019
@njhale
Copy link
Member Author

njhale commented Mar 13, 2019

/retest

njhale added 6 commits March 15, 2019 09:49
@njhale
chore(modules): revendor
28ffdc0
@njhale
style(scripts): rename package-release.sh to package_release.sh
0d98d3d
@njhale
refactor(scripts): use existing manifests in package_quickstart.sh
60ce3d3 
- Rename package-quickstart.sh to package_quickstart.sh
- Use existing manifests in package_quickstart.sh instead of helm
@njhale
chore(docker): update upstream.Dockerfile base to go 1.12
8a49e62 
- Updates upstream.Dockerfile base to go 1.12
- Removes alm-ci stage
@njhale
feat(make): add quickstart generation to release rule
b7bbf0c 
- Add quickstart generation to release rule
- Use operator-framework namespace for all image references
- Support go 1.12 go modules flag
@njhale
refactor(images): use operator-framework namespace for all olm images
a017baa
@njhale njhale force-pushed the change-upstream-base branch from 04b4d30 to a017baa Compare March 15, 2019 13:51
@ecordell
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 15, 2019
alecmerdler
alecmerdler approved these changes Mar 15, 2019
View reviewed changes
Copy link
Member

@alecmerdler alecmerdler 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
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alecmerdler, ecordell, njhale

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [alecmerdler,ecordell,njhale]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 4d6bb6c into operator-framework:master Mar 15, 2019
@ecordell ecordell added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 19, 2019
@njhale njhale mentioned this pull request Mar 21, 2019
Closed
@njhale njhale deleted the change-upstream-base branch September 30, 2019 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ecordell ecordell ecordell approved these changes

@alecmerdler alecmerdler alecmerdler approved these changes

Assignees

@ecordell ecordell

@alecmerdler alecmerdler

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@njhale @alecmerdler @ecordell @openshift-ci-robot @mr24091997 @openshift-merge-robot