Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

change controller finalizer back to kubernetes-incubator #2664

Merged
merged 1 commit into from
Jun 18, 2019

Conversation

jberkhahn
Copy link
Contributor

as per the discussion in the meeting today

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 17, 2019
@jberkhahn
Copy link
Contributor Author

/cc @mszostok

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 17, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jberkhahn

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 17, 2019
@mszostok
Copy link
Contributor

/test pull-service-catalog-xbuild

@mszostok
Copy link
Contributor

found one thing:

you should also change the test data, it's mocked (both returned responses and golden files), so tests didn't catch that by IMO we should also rename that to do not confuse others.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 17, 2019
@jberkhahn
Copy link
Contributor Author

@mszostok that should fix it

@jberkhahn
Copy link
Contributor Author

/test pull-service-catalog-unit

@MHBauer
Copy link
Contributor

MHBauer commented Jun 18, 2019

yea, this would definitely not be a compatible change.
Unfortunately, it probably means needing to have two code paths for a while.
Can brainstorm later if you want.

@mszostok
Copy link
Contributor

mszostok commented Jun 18, 2019

@MHBauer during the SIG meeting we discuss to move this change to CRD migration tool.

Here is the migration scenario from api-server to CRDs: https://github.com/kyma-incubator/service-catalog/blob/crds-migration/docs/migration-apiserver-to-crds.md#details-of-an-upgrade-and-migration

so we can add just one step to replace the finalizer name before saving resources into etcd and starting the new controller image.

That should be quite easy to do, and additionally, we will have only one migration process instead of supporting two of them, wdyt?

@mszostok
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 18, 2019
@k8s-ci-robot k8s-ci-robot merged commit 0de5bec into kubernetes-retired:master Jun 18, 2019
viviyww pushed a commit to viviyww/service-catalog that referenced this pull request Jun 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants