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

encryption: switch to kube-storage-version-migrator #713

Merged
merged 2 commits into from Jan 24, 2020
Merged

encryption: switch to kube-storage-version-migrator #713

merged 2 commits into from Jan 24, 2020

Conversation

vareti
Copy link
Contributor

@vareti vareti commented Jan 9, 2020

continuing the work done in #637

Requires https://github.com/openshift/cluster-kube-storage-version-migrator-operator to be in the release.

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 9, 2020
sttts
sttts reviewed Jan 14, 2020
View reviewed changes
pkg/operator/starter.go Outdated Show resolved Hide resolved
@vareti
Copy link
Contributor Author

vareti commented Jan 14, 2020

/test e2e-aws

@vareti vareti changed the title encryption: switch to kube-storage-version-migrator [WIP] encryption: switch to kube-storage-version-migrator Jan 14, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 14, 2020
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 14, 2020
@vareti
Copy link
Contributor Author

vareti commented Jan 14, 2020

/retest

@vareti
Copy link
Contributor Author

vareti commented Jan 14, 2020

/test e2e-aws

@vareti
Copy link
Contributor Author

vareti commented Jan 15, 2020

/retest

@vareti vareti changed the title [WIP] encryption: switch to kube-storage-version-migrator encryption: switch to kube-storage-version-migrator Jan 15, 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 Jan 15, 2020
sttts
sttts reviewed Jan 15, 2020
View reviewed changes
pkg/operator/starter.go Outdated

migrationClientConfig := dynamic.ConfigFor(controllerContext.KubeConfig)
migrationClientConfig.Burst = 40
migrationClientConfig.QPS = 30
Copy link
Contributor

Choose a reason for hiding this comment

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

this client is just for the CRD. No need to tweak the values. Instead we have to verify that kube-storage-version-migrator does the same tweaking internally.

@sanchezl sanchezl mentioned this pull request Jan 15, 2020
Closed
@sanchezl
Copy link
Contributor

/approve

@sanchezl
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 15, 2020
@sanchezl sanchezl mentioned this pull request Jan 17, 2020
Closed
8 tasks
@sttts
Copy link
Contributor

sttts commented Jan 20, 2020

/test e2e-aws-operator-encryption

@sttts
Copy link
Contributor

sttts commented Jan 20, 2020

/approve
/hold

Want to see the e2e-aws-operator-encryption test before merging.

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 20, 2020
@p0lyn0mial
Copy link
Contributor

/test e2e-aws-operator-encryption-perf

@p0lyn0mial
Copy link
Contributor

running /test e2e-aws-operator-encryption-perf because it keeps failing due to some infra issues.

@vareti
Copy link
Contributor Author

vareti commented Jan 22, 2020

/test e2e-aws-operator-encryption-perf

7 similar comments
@vareti
Copy link
Contributor Author

vareti commented Jan 22, 2020

/test e2e-aws-operator-encryption-perf

@sttts
Copy link
Contributor

sttts commented Jan 22, 2020

/test e2e-aws-operator-encryption-perf

@vareti
Copy link
Contributor Author

vareti commented Jan 22, 2020

/test e2e-aws-operator-encryption-perf

@vareti
Copy link
Contributor Author

vareti commented Jan 22, 2020

/test e2e-aws-operator-encryption-perf

@vareti
Copy link
Contributor Author

vareti commented Jan 22, 2020

/test e2e-aws-operator-encryption-perf

@sanchezl
Copy link
Contributor

/test e2e-aws-operator-encryption-perf

@vareti
Copy link
Contributor Author

vareti commented Jan 23, 2020

/test e2e-aws-operator-encryption-perf

@vareti
Copy link
Contributor Author

vareti commented Jan 23, 2020

/test e2e-aws-operator-encryption-perf

It still hits the same AMI errors. not sure if something needs to be done from PR side.

@p0lyn0mial
Copy link
Contributor

/test e2e-aws-operator-encryption-perf

@vareti
Copy link
Contributor Author

vareti commented Jan 23, 2020

/test e2e-aws-operator-encryption

@vareti
Copy link
Contributor Author

vareti commented Jan 23, 2020

/test e2e-aws-operator-encryption-perf

@sttts
Copy link
Contributor

sttts commented Jan 23, 2020

/retest

@vareti
Copy link
Contributor Author

vareti commented Jan 23, 2020

/test e2e-aws

1 similar comment
@vareti
Copy link
Contributor Author

vareti commented Jan 23, 2020

/test e2e-aws

@vareti
Copy link
Contributor Author

vareti commented Jan 23, 2020

/retest

@sttts
Copy link
Contributor

sttts commented Jan 23, 2020

/retest
/hold cancel

@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 Jan 23, 2020
@vareti
Copy link
Contributor Author

vareti commented Jan 24, 2020

/test e2e-aws
/test e2e-aws-operator-encryption
/test e2e-aws-operator-encryption-perf

@vareti
Copy link
Contributor Author

vareti commented Jan 24, 2020

/test e2e-aws

@p0lyn0mial
Copy link
Contributor

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: p0lyn0mial, sanchezl, sttts, vareti

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

@openshift-merge-robot openshift-merge-robot merged commit 7e19f69 into openshift:master Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts sttts left review comments

@sanchezl sanchezl sanchezl left review comments

@p0lyn0mial p0lyn0mial p0lyn0mial left review comments

@mfojtik mfojtik Awaiting requested review from mfojtik

@soltysh soltysh Awaiting requested review from soltysh

Assignees

@sanchezl sanchezl

@p0lyn0mial p0lyn0mial

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/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.

None yet
6 participants
@vareti @sanchezl @sttts @p0lyn0mial @openshift-ci-robot @openshift-merge-robot