WIP: Add machineconfiguration API by LorbusChris · Pull Request #301 · openshift/api

LorbusChris · 2019-04-29T17:09:48Z

This PR moves over the type definitions for the machineconfiguration API from the MCO repo, except for those depending on Ignition, which will stay in the MCO repo.

WIP

cc @runcom @sttts

LorbusChris · 2019-04-30T13:08:13Z

Any hint is appreciated! Please also consider @kikisdeliveryservice's comments over at runcom#1 (comment)

sttts · 2019-05-06T13:28:19Z

Please double check (missing) +required tags and omitempty.

LorbusChris · 2019-05-06T14:34:40Z

Please double check (missing) +required tags and omitempty.

Where can I deduce these? cc @runcom

LorbusChris · 2019-05-06T15:18:27Z

~~Build should be succeeding now 🎉~~ only locally :/

What's still missing is this part:

Please double check (missing) +required tags and omitempty.

LorbusChris · 2019-05-06T15:21:48Z

/test verify

LorbusChris · 2019-05-06T19:41:56Z

Running hack/update-protobuf.sh locally as suggested by prow yields no changes (as expected that is, b/c I already ran and commited make generate which includes this).

Why is it still not working now?

LorbusChris · 2019-05-07T11:43:14Z

/retest

openshift-ci-robot · 2019-06-13T13:46:29Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LorbusChris
To complete the pull request process, please assign derekwaynecarr
You can assign the PR to them by writing /assign @derekwaynecarr in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

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

LorbusChris · 2019-06-20T15:25:36Z

This is ready for another round of reviews

soltysh · 2019-06-21T09:56:44Z

+	CloudProviderConfig string `json:"cloudProviderConfig"`
+
+	// The openshift platform, e.g. "libvirt", "openstack", "aws", or "none"
+	Platform string `json:"platform"`


Do right away, why followup, same rules applies.

soltysh · 2019-06-21T09:57:52Z

+	// etcdDiscoveryDomain specifies the etcd discovery domain
+	EtcdDiscoveryDomain string `json:"etcdDiscoveryDomain"`
+
+	// TODO: Investigate using string for CA data


string is being used in other places, so I'd prefer we keep it consistent.

Afaiu v1 has already been released, albeit from the mco repo, and not this one. Can I still change this then?

same goes for the KubeletConfig type below

As we talked about: the use of RawExtensions instead of an explicit kubelet config type will not change the wire format. So that's possible and should be done.

soltysh · 2019-06-21T10:00:34Z

+
+	// pullSecret is the default pull secret that needs to be installed
+	// on all machines.
+	PullSecret *corev1.ObjectReference `json:"pullSecret,omitempty"`


Have you considered using ConfigMapFileReference or ConfigMapNameReference or SecretNameReference from https://github.com/openshift/api/blob/master/config/v1/types.go ?

same question as above: Can we still change this for v1 at this point? Do we want to?
cc @runcom @cgwalters

soltysh · 2019-06-21T10:02:12Z

+
+	// osImageURL is the location of the container image that contains the OS update payload.
+	// It is sourced from configmap/machine-config-osimageurl
+	OSImageURL string `json:"osImageURL"`


What do you mean sourced from if it's a reference why not using ConfigMapNameReference ?

The string value is taken from cm/machine-config-osimageurl.data.osImageURL

soltysh · 2019-06-21T10:06:49Z

+// KubeletConfigSpec defines the desired state of KubeletConfig
+type KubeletConfigSpec struct {
+	MachineConfigPoolSelector *metav1.LabelSelector                      `json:"machineConfigPoolSelector,omitempty"`
+	KubeletConfig             *kubeletconfigv1beta1.KubeletConfiguration `json:"kubeletConfig,omitempty"`


You're exposing this as part of v1 API, postponing means you won't get a chance to do it, ever.

LorbusChris · 2019-06-25T19:29:00Z

@soltysh I don't think we can change the types here now, as we have already released this API as v1 from the MCO repo (this PR is just a move). Please anybody correct me if I am wrong on this. cc @runcom @cgwalters @sttts .

I have addressed the other requested changes.

This commit moves over the definitions of the following resource types in machineconfiguration/v1 from the openshift/machine-config-operator repository: - ContainerRuntimeConfig - ControllerConfig - KubeletConfig - MachineConfigPool

Run `hack/update-deps.sh && make generate-with-container`

deads2k · 2019-09-25T18:18:20Z

Christian Glombek
@deads2k that's currently blocked because our types depend on ignition's types internally. we want to move to RawExtension first, before we move it over to the api repo.

/close

Feel free to re-open once that's updated.

openshift-ci-robot · 2019-09-25T18:18:22Z

@deads2k: Closed this PR.

Details

In response to this:

Christian Glombek
@deads2k that's currently blocked because our types depend on ignition's types internally. we want to move to RawExtension first, before we move it over to the api repo.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 29, 2019

openshift-ci-robot requested review from abhinavdahiya and enj April 29, 2019 17:10

LorbusChris mentioned this pull request Apr 29, 2019

WIP: Add machineconfiguration API runcom/api#1

Closed

LorbusChris commented Apr 29, 2019

View reviewed changes

Comment thread hack/lib/init.sh Outdated

LorbusChris commented Apr 29, 2019

View reviewed changes

Comment thread hack/lib/init.sh Outdated

runcom mentioned this pull request May 3, 2019

Add clusteroperators and clusterversions to explain tests openshift/origin#22748

Merged

sttts reviewed May 6, 2019

View reviewed changes

Comment thread glide.yaml Outdated

sttts reviewed May 6, 2019

View reviewed changes

Comment thread machineconfiguration/v1/doc.go Outdated

sttts reviewed May 6, 2019

View reviewed changes

Comment thread machineconfiguration/v1/doc.go Outdated

sttts reviewed May 6, 2019

View reviewed changes

Comment thread machineconfiguration/v1/types.go Outdated

LorbusChris force-pushed the machineconfig branch from 4522a1a to 5853953 Compare May 6, 2019 14:32

LorbusChris force-pushed the machineconfig branch from 5853953 to 81a1eb0 Compare May 6, 2019 15:14

LorbusChris force-pushed the machineconfig branch 4 times, most recently from 170f481 to 060ad22 Compare May 6, 2019 19:34

LorbusChris force-pushed the machineconfig branch 3 times, most recently from 060ad22 to 6f704f4 Compare May 7, 2019 12:03

sttts reviewed May 7, 2019

View reviewed changes

Comment thread glide.yaml Outdated

sttts reviewed May 7, 2019

View reviewed changes

Comment thread authorization/v1/generated.pb.go Outdated

openshift-ci-robot requested review from cgwalters and runcom June 13, 2019 13:42

LorbusChris force-pushed the machineconfig branch from 1c6b4a5 to a3c7caf Compare June 13, 2019 13:46

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2019

LorbusChris changed the title ~~Add machineconfiguration API~~ [WIP] Add machineconfiguration API Jun 13, 2019

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 Jun 13, 2019

LorbusChris force-pushed the machineconfig branch 4 times, most recently from e886845 to b9fac8e Compare June 14, 2019 17:18

LorbusChris changed the title ~~[WIP] Add machineconfiguration API~~ Add machineconfiguration API Jun 14, 2019

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 Jun 14, 2019

LorbusChris force-pushed the machineconfig branch 2 times, most recently from 400c23a to 4535328 Compare June 20, 2019 14:47

soltysh suggested changes Jun 21, 2019

View reviewed changes

LorbusChris force-pushed the machineconfig branch from 4535328 to d7c4761 Compare June 25, 2019 19:05

LorbusChris added 2 commits June 25, 2019 22:22

Update dependencies and regenerate

35920bb

Run `hack/update-deps.sh && make generate-with-container`

LorbusChris force-pushed the machineconfig branch from d7c4761 to 35920bb Compare June 25, 2019 20:54

LorbusChris mentioned this pull request Jul 3, 2019

API Updates openshift/machine-config-operator#898

Merged

LorbusChris changed the title ~~Add machineconfiguration API~~ WIP: Add machineconfiguration API Jul 3, 2019

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 Jul 3, 2019

openshift-ci-robot closed this Sep 25, 2019

kikisdeliveryservice mentioned this pull request Oct 22, 2021

[WIP] move mco api to openshift/api #1040

Closed

jkyros mentioned this pull request May 4, 2023

MCO-52: add MCO API into openshift/api #1453

Merged

Conversation

LorbusChris commented Apr 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LorbusChris commented Apr 30, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sttts commented May 6, 2019

Uh oh!

Uh oh!

LorbusChris commented May 6, 2019

Uh oh!

LorbusChris commented May 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LorbusChris commented May 6, 2019

Uh oh!

LorbusChris commented May 6, 2019

Uh oh!

LorbusChris commented May 7, 2019

Uh oh!

Uh oh!

Uh oh!

openshift-ci-robot commented Jun 13, 2019

Uh oh!

LorbusChris commented Jun 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LorbusChris Jun 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LorbusChris commented Jun 25, 2019

Uh oh!

deads2k commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Sep 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

LorbusChris commented Apr 29, 2019 •

edited

Loading

LorbusChris commented May 6, 2019 •

edited

Loading

LorbusChris commented Jun 20, 2019 •

edited

Loading

LorbusChris Jun 25, 2019 •

edited

Loading

deads2k commented Sep 25, 2019 •

edited

Loading