Accomodate provider spec #104

ingvagabund · 2018-12-23T16:54:08Z

Built on top of #103

Due to the following breaking change kubernetes-sigs/cluster-api#548 we need to support both .providerConfig and .providerSpec fields until all consumers of the libvirt actuator migrate to new .providerSpec field.

We could just keep the .providerConfig and make sure it gets de-serialized into ProviderSpec struct. Though, folks could get confused by that and we should stay with the upstream name convention.

Once merged, we can rename the following:

providerConfig
ProviderConfig
DecodeProviderConfig
EncodeProviderConfig
LibvirtMachineProviderConfig

to corresponding:

providerSpec
ProviderSpec
DecodeProviderSpec
EncodeProviderSpec
LibvirtMachineProviderSpec

plus other identifiers that use provider{c,C}onfig string.

cynepco3hahue · 2018-12-24T07:54:33Z

Gopkg.toml

@@ -20,10 +20,11 @@ required = [

 [[constraint]]
  name = "sigs.k8s.io/cluster-api"
+  #revision = "0734939e05aeb64ab198e3feeee8b4e90ee5cbb2"
  branch = "master"


I think it better to use tag 0.0.0-alpha.4 because it includes all recent changes and will prevent broken API on deps update.
The same true for all related cluster-api repositories.

That release does not contain kubernetes-sigs/cluster-api#598. It's been unwritten rule to take a specific revision or master HEAD til now. Given we usually need the latest changes in the project, taking anything than master HEAD can be problematic.

I see your point but can we at least stick to some commit on the master?

If we do it here, we need to do it everywhere else. Though, we can wait until the next release is out and update all deps at once in all repos.

Otherwise, the exact sha of the cluster-api is in the Godep.lock file. It gets picked by the dep itself.

ingvagabund · 2018-12-30T15:51:28Z

/retest

paulfantom · 2019-01-03T12:06:03Z

/retest

paulfantom · 2019-01-03T15:40:39Z

cmd/libvirt-actuator/main.go

@@ -309,6 +309,12 @@ func bootstrapCommand() *cobra.Command {
 				glog.Info(msg)
 			}

+			clusterFramework.MachineControllerImage = "openshift/origin-libvirt-machine-controllers:v4.0.0"
+			clusterFramework.MachineManagerImage = "openshift/origin-libvirt-machine-controllers:v4.0.0"
+			// TODO(jchaloup): once docker.io/openshift/origin-machine-api-operator gets updated, switch back to it.


It should get an update today, as openshift/machine-api-operator#159 is on its way.

Who is responsible for making the updates?

prow, this line: https://github.com/openshift/release/blob/master/cluster/ci/config/mirroring/origin_v4_0#L54

How often is it done?

As far as I remember it should be done every couple of hours

The latest mao docker images is two weeks old :-|

$ docker pull docker.io/openshift/origin-machine-api-operator $ docker images | grep docker.io/openshift/origin-machine-api-operator docker.io/openshift/origin-machine-api-operator latest 6a73aff2da22 2 weeks ago 357 MB

It seems like like CI image mirroring responsible for uploading images to docker hub was down between ~13 days ago and ~43 minutes ago.

paulfantom · 2019-01-03T15:42:51Z

/approve

putting it on hold since we should have a new mao image in docker hub today (this should do it: openshift/machine-api-operator#159) and in turn new image should resolve a TODO.

/hold

openshift-ci-robot · 2019-01-03T15:43:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: paulfantom

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~~ [paulfantom]

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

ingvagabund · 2019-01-03T15:53:47Z

/retest

Makefile

pkg/cloud/libvirt/actuators/machine/actuator.go

Make sure .providerConfig field of machine object is still available until all consumers migrate to .providerSpec field. The .providerSpec field is the default field now and .providerConfig is used only when both .providerSpec.Value and .providerSpec.ValueFrom are nil.

frobware · 2019-01-04T14:31:34Z

/lgtm

ingvagabund · 2019-01-04T15:44:24Z

/hold cancel
we can switch to the docker image at any point.

Since we merge this upstream https://github.com/kubernetes-sigs/cluster-api/pull/593/files we needed to support both annotations for backwards compatibility. After aws/libvirt actuators including latest cluster-api, the old annotation can be removed. See openshift/cluster-api-provider-aws#127 and openshift/cluster-api-provider-libvirt#104

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.

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.

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 23, 2018

openshift-ci-robot requested review from frobware and paulfantom December 23, 2018 16:54

ingvagabund mentioned this pull request Dec 23, 2018

Support both providerConfig and providerSpec under machine CRD's openshift/machine-api-operator#155

Merged

2 tasks

cynepco3hahue reviewed Dec 24, 2018

View reviewed changes

ingvagabund added 3 commits January 3, 2019 16:33

Bump cluster-api to the latest master HEAD

1740a56

Apply downstream patches

619142c

Sync github.com/openshift/cluster-api-actuator-pkg

dc87c58

paulfantom reviewed Jan 3, 2019

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 3, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 3, 2019

frobware requested changes Jan 4, 2019

View reviewed changes

Makefile Show resolved Hide resolved

pkg/cloud/libvirt/actuators/machine/actuator.go Outdated Show resolved Hide resolved

pkg/cloud/libvirt/actuators/machine/actuator.go Outdated Show resolved Hide resolved

frobware approved these changes Jan 4, 2019

View reviewed changes

openshift-ci-robot assigned frobware Jan 4, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 4, 2019

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 4, 2019

openshift-merge-robot merged commit da6aa0a into openshift:master Jan 4, 2019

ingvagabund deleted the accomodate-provider-spec branch January 4, 2019 23:37

ingvagabund mentioned this pull request Jan 6, 2019

Support both .providerConfig and .providerSpec fields openshift/machine-api-operator#161

Closed

enxebre mentioned this pull request Jan 7, 2019

Remove deprecated machine annotation openshift/machine-api-operator#162

Merged

enxebre mentioned this pull request Jan 9, 2019

Providerspec openshift/installer#1027

Merged

enxebre mentioned this pull request Jan 10, 2019

Drop deprecated providerConfig field openshift/machine-api-operator#170

Closed

enxebre mentioned this pull request Jan 10, 2019

Crds drop old field openshift/machine-api-operator#173

Merged

ingvagabund mentioned this pull request Jan 11, 2019

Rename ProviderConfig to ProviderSpec openshift/kubernetes-autoscaler#22

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accomodate provider spec #104

Accomodate provider spec #104

ingvagabund commented Dec 23, 2018

cynepco3hahue Dec 24, 2018 •

edited

ingvagabund Dec 30, 2018

cynepco3hahue Jan 2, 2019

ingvagabund Jan 3, 2019

ingvagabund Jan 3, 2019 •

edited

ingvagabund commented Dec 30, 2018

paulfantom commented Jan 3, 2019

paulfantom Jan 3, 2019

ingvagabund Jan 3, 2019

paulfantom Jan 3, 2019 •

edited

ingvagabund Jan 3, 2019

paulfantom Jan 3, 2019

ingvagabund Jan 3, 2019 •

edited

paulfantom Jan 3, 2019

paulfantom commented Jan 3, 2019

openshift-ci-robot commented Jan 3, 2019

ingvagabund commented Jan 3, 2019

frobware commented Jan 4, 2019

ingvagabund commented Jan 4, 2019

Accomodate provider spec #104

Accomodate provider spec #104

Conversation

ingvagabund commented Dec 23, 2018

cynepco3hahue Dec 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ingvagabund Jan 3, 2019 • edited

Choose a reason for hiding this comment

ingvagabund commented Dec 30, 2018

paulfantom commented Jan 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulfantom Jan 3, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ingvagabund Jan 3, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulfantom commented Jan 3, 2019

openshift-ci-robot commented Jan 3, 2019

ingvagabund commented Jan 3, 2019

frobware commented Jan 4, 2019

ingvagabund commented Jan 4, 2019

cynepco3hahue Dec 24, 2018 •

edited

ingvagabund Jan 3, 2019 •

edited

paulfantom Jan 3, 2019 •

edited

ingvagabund Jan 3, 2019 •

edited