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.

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

Rename ProviderConfig to ProviderSpec #548

Merged
merged 1 commit into from
Nov 21, 2018

Conversation

dlipovetsky
Copy link
Contributor

What this PR does / why we need it:
Renames ProviderConfig to ProviderSpec, to match the Spec/Status convention used in Kubernetes-style APIs.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #414

Special notes for your reviewer:
This is a breaking API change. The consensus in the meeting a couple months back was for in support of this change, but it can be put on hold for a while. I will test this change with https://github.com/kubernetes-sigs/cluster-api-provider-gcp/ and report back in this PR.

Release Note:

Breaking API Change: ProviderConfig renamed to ProviderSpec (#414)

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 17, 2018
@alvaroaleman
Copy link
Member

Hm. Any chance we can increment to a v1alpha2 when merging this?

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 17, 2018
@roberthbailey
Copy link
Contributor

@alvaroaleman - why do you want to increment the api version? Right now, with a single api version, we don't have to worry about version translation, which isn't yet a feature of CRDs. Are you suggesting that we move to v1alpha2 and drop v1alpha1?

@roberthbailey
Copy link
Contributor

/lgtm
/approve
/hold

Holding on resolution of @alvaroaleman's question.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 18, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dlipovetsky, roberthbailey

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 Oct 18, 2018
@alvaroaleman
Copy link
Member

@roberthbailey

why do you want to increment the api version? Right now, with a single api version, we don't have to worry about version translation, which isn't yet a feature of CRDs. Are you suggesting that we move to v1alpha2 and drop v1alpha1?

Yes exactly that, to a) make everyone aware there is a pretty breaking change b) allow folks to build their own migration if they choose to

@roberthbailey
Copy link
Contributor

@dlipovetsky - are you ok waiting to merge this until we can discuss during the weekly meeting next Wed about changing the apiversion?

@dlipovetsky
Copy link
Contributor Author

@roberthbailey Absolutely ok with discussing it at the meeting, thanks. I'll also try to verify using the GCP provider before then.

@roberthbailey roberthbailey added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2018
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 27, 2018
@dlipovetsky
Copy link
Contributor Author

I rebased. As part of the rebase, I renamed Config->Spec in the newly added MachineClass types.

@dlipovetsky
Copy link
Contributor Author

@roberthbailey I think my plan to try out this change with the GCP provider was too ambitious. I started working on this, but I noticed that the GCP provider was vendoring https://github.com/pwittrock/cluster-api, and I'm not changing to my branch will be straightforward.

Is it reasonable to merge this PR and let providers update later?

@roberthbailey
Copy link
Contributor

I sent kubernetes-sigs/cluster-api-provider-gcp#49 to update the vendored dependency.

But either way, the only real way to test your change in that repo is to change the source though. You need to add a constraint to Gopkg.toml locally, run dep ensure -update sigs.k8s.io/cluster-api to pull in your changes, and then you can test. Something like

[[constraint]]	
  name="sigs.k8s.io/cluster-api"	
  revision="insert git hash here"	
  source="https://github.com/dlipovetsky/cluster-api"

should do the trick.

@dlipovetsky
Copy link
Contributor Author

@roberthbailey Thanks! I was less worried about dep, and more worried about the delta between pwittrock's branch and mine, which is rebased on master. Once your kubernetes-sigs/cluster-api-provider-gcp#49 lands, I'll try building gcp provider with my changes here.

@roberthbailey
Copy link
Contributor

The PR in the gcp repo just merged.

@roberthbailey roberthbailey removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 3, 2018
@roberthbailey
Copy link
Contributor

@dlipovetsky - have you had a chance to test yet?

We'd like to get this merged soon since the gitbook documentation is written assuming that this change has already gone in. :)

enxebre added a commit to enxebre/machine-api-operator that referenced this pull request Jan 10, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548. AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127 and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner openshift#155, hence we want to only allow the new field here and eventually drop the old one on the providers.
enxebre added a commit to enxebre/machine-api-operator that referenced this pull request Jan 10, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548. AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127 and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner openshift#155, hence we want to only allow the new field here and eventually drop the old one on the providers.
enxebre added a commit to enxebre/machine-api-operator that referenced this pull request Jan 10, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548. AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127 and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner openshift#155, hence we want to only allow the new field here and eventually drop the old one on the providers.
enxebre added a commit to enxebre/machine-api-operator that referenced this pull request Jan 10, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548. AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127 and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner openshift#155, hence we want to only allow the new field here and eventually drop the old one on the providers.
ingvagabund added a commit to ingvagabund/autoscaler that referenced this pull request Jan 11, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 18, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 20, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 26, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 27, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Mar 27, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
smarterclayton pushed a commit to smarterclayton/autoscaler that referenced this pull request Mar 29, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
frobware pushed a commit to frobware/autoscaler that referenced this pull request Jun 19, 2019
The machine provideConfig field has been renamed upstream kubernetes-sigs/cluster-api#548.
AWS/Libvirt providers have been accommodated for supporting the new field openshift/cluster-api-provider-aws#127
and openshift/cluster-api-provider-libvirt#104 in a backward compatible manner.
Hence we want to only allow the new field here and eventually drop the old one on the providers.
foot pushed a commit to weaveworks/weave-gitops-enterprise that referenced this pull request Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 "Looks good to me", 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.

Change ProviderConfig to ProviderSpec
4 participants