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

Remove package server subscription #942

Merged
merged 3 commits into from
Jul 12, 2019
Merged

Remove package server subscription #942

merged 3 commits into from
Jul 12, 2019

Conversation

ecordell
Copy link
Member

@ecordell ecordell commented Jul 3, 2019

This PR includes #931 but also simplifies our deploy manifests by removing the catalog and subscription for package-server.

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 3, 2019
@ecordell
Copy link
Member Author

ecordell commented Jul 3, 2019

/retest

1 similar comment
@ecordell
Copy link
Member Author

ecordell commented Jul 3, 2019

/retest

@ecordell
Copy link
Member Author

ecordell commented Jul 3, 2019

/test e2e-aws-upgrade

@ecordell
Copy link
Member Author

ecordell commented Jul 3, 2019

/test e2e-aws

@ecordell
Copy link
Member Author

ecordell commented Jul 3, 2019

/refresh

@ecordell
Copy link
Member Author

ecordell commented Jul 3, 2019

/retest

2 similar comments
@ecordell
Copy link
Member Author

ecordell commented Jul 4, 2019

/retest

@ecordell
Copy link
Member Author

ecordell commented Jul 4, 2019

/retest

@njhale
Copy link
Member

njhale commented Jul 4, 2019 •
edited
Loading

e2e-aws is failing because an OLM OpenShift conformance test expects that at least one instance of each OLM kind exists on a given cluster. We could potentially ask @jianzhangbjz to remove or modify this expectation in openshift/origin#23224.

@njhale njhale mentioned this pull request Jul 5, 2019
Closed
njhale
njhale reviewed Jul 5, 2019
View reviewed changes
Copy link
Member

@njhale njhale left a comment

Choose a reason for hiding this comment

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

Looks good! Just one question:

manifests/0000_50_olm_15-packageserver.clusterserviceversion.yaml
@@ -9,7 +9,6 @@ spec:
displayName: Package Server
description: Represents an Operator package that is available from a given CatalogSource which will resolve to a ClusterServiceVersion.
minKubeVersion: 1.11.0
replaces: packageserver.v0.9.0
Copy link
Member

@njhale njhale Jul 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Why remove this?

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 updated the init to delete packageserver.v0.9.0 and packageserver.v0.10.0 - so whatever was there before, we're now moving to just having a single packageserver that we can update.

Copy link
Member

Choose a reason for hiding this comment

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

are there clusters that still have 0.10.1?

@ecordell
Copy link
Member Author

ecordell commented Jul 8, 2019

/retest

1 similar comment
@ecordell
Copy link
Member Author

ecordell commented Jul 9, 2019

/retest

@ecordell ecordell force-pushed the csv-packageserver branch from f03cdcd to cad128e Compare July 9, 2019 17:01
@ecordell
Copy link
Member Author

ecordell commented Jul 9, 2019

/retest

@ecordell ecordell force-pushed the csv-packageserver branch from cad128e to 5fb4811 Compare July 10, 2019 13:22
@ecordell
Copy link
Member Author

/retest

@ecordell ecordell force-pushed the csv-packageserver branch from 96dbe5d to fd24ed1 Compare July 10, 2019 21:35
@ecordell
Copy link
Member Author

/retest

2 similar comments
@ecordell
Copy link
Member Author

/retest

@ecordell
Copy link
Member Author

/retest

ecordell added 2 commits July 11, 2019 10:58
@ecordell
chore(deploy): remove packageserver subscription and catalog from chart
81801f1 
this has several advantages:
 - deploy artifacts are simpler
 - faster install/startup time
 - simpler integration with CVO
 - unblocking using kustomize for our release scripts
@ecordell
feat(init): add init container to olm operator that can clean previous
9564623 
deployments

CVO doesn't delete manifests during upgrades, so we need to clean up
after older releases.
@ecordell ecordell force-pushed the csv-packageserver branch from fd24ed1 to bd63c8f Compare July 11, 2019 15:26
@ecordell
Copy link
Member Author

/retest

1 similar comment
@ecordell
Copy link
Member Author

/retest

@ecordell
fix(init): move cleanup to olm startup
38012d8 
init container is not getting a serviceaccount token mounted for some
reason, this sidesteps the issue.

previous commit remains in case we can switch back to an init container
at some point
@ecordell ecordell force-pushed the csv-packageserver branch from bd63c8f to 38012d8 Compare July 11, 2019 22:44
njhale
njhale approved these changes Jul 12, 2019
View reviewed changes
Copy link
Member

@njhale njhale left a comment

Choose a reason for hiding this comment

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

Sweet!

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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

@ecordell
Copy link
Member Author

/retest

1 similar comment
@ecordell
Copy link
Member Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit 4b7a384 into operator-framework:master Jul 12, 2019
@ecordell ecordell deleted the csv-packageserver branch July 12, 2019 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@njhale njhale njhale approved these changes

@alecmerdler alecmerdler Awaiting requested review from alecmerdler

@jpeeler jpeeler Awaiting requested review from jpeeler

Assignees

@njhale njhale

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

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