BUG 1816624: MHC structural schema #536

enxebre · 2020-03-24T18:28:45Z

To generate CRDs with structural schema compliant properties we update sigs.k8s.io/controller-tools
As a consequence we need update pretty much all the k8s and openshift api machinery stack to kube 1.18. Controller-runtime points to kubernetes-sigs/controller-runtime#836.
This also force us to update to go 1.13. Needs openshift/release#7889

Also as a consequence of the kube 1.18 update we are forced to take the chance to finally drop the CVO lib deps in favour of openshift/library-go which uses the latest versions of the API machinery.

After regenerating the CRDs I manually added preserveUnknownFields:false for MHC.

kubectl explain mhc --recursive=true
KIND:     MachineHealthCheck
VERSION:  machine.openshift.io/v1beta1

DESCRIPTION:
     MachineHealthCheck is the Schema for the machinehealthchecks API

FIELDS:
   apiVersion	<string>
   kind	<string>
   metadata	<Object>
      annotations	<map[string]string>
      clusterName	<string>
      creationTimestamp	<string>
      deletionGracePeriodSeconds	<integer>
      deletionTimestamp	<string>
      finalizers	<[]string>
      generateName	<string>
      generation	<integer>
      labels	<map[string]string>
      managedFields	<[]Object>
         apiVersion	<string>
         fieldsType	<string>
         fieldsV1	<map[string]>
         manager	<string>
         operation	<string>
         time	<string>
      name	<string>
      namespace	<string>
      ownerReferences	<[]Object>
         apiVersion	<string>
         blockOwnerDeletion	<boolean>
         controller	<boolean>
         kind	<string>
         name	<string>
         uid	<string>
      resourceVersion	<string>
      selfLink	<string>
      uid	<string>
   spec	<Object>
      maxUnhealthy	<>
      nodeStartupTimeout	<string>
      selector	<Object>
         matchExpressions	<[]Object>
            key	<string>
            operator	<string>
            values	<[]string>
         matchLabels	<map[string]string>
      unhealthyConditions	<[]Object>
         status	<string>
         timeout	<string>
         type	<string>
   status	<Object>
      currentHealthy	<integer>
      expectedMachines	<integer>

To generate CRDs with structural schema compliant properties we update sigs.k8s.io/controller-tools This indirectly requires to update pretty much all the k8s and openshift api machinery stack. k8s.io/api v0.18.0-rc.1 k8s.io/apimachinery v0.18.0-rc.1 k8s.io/client-go v0.18.0-rc.1 k8s.io/code-generator v0.18.0-rc.1 k8s.io/klog v1.0.0 k8s.io/kube-aggregator v0.18.0-rc.1 // indirect k8s.io/kubectl v0.18.0-rc.1 github.com/openshift/api v0.0.0-20200323095748-e7041f8762a3 github.com/openshift/client-go v0.0.0-20200320150128-a906f3d8e723 github.com/openshift/library-go v0.0.0-20200324092245-db2a8546af81

…support after adding kube 1.18 deps

openshift-ci-robot · 2020-03-24T18:28:55Z

@enxebre: This pull request references Bugzilla bug 1816624, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

BUG 1816624: MHC structural schema

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 · 2020-03-24T18:30:17Z

@enxebre: This pull request references Bugzilla bug 1816624, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

BUG 1816624: MHC structural schema

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.

enxebre · 2020-03-24T19:03:17Z

/cc @JoelSpeed

openshift-ci-robot · 2020-03-24T19:20:09Z

@enxebre: This pull request references Bugzilla bug 1816624, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

BUG 1816624: MHC structural schema

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.

enxebre · 2020-03-25T10:10:38Z

/retest

enxebre · 2020-03-25T10:21:07Z

/test images

enxebre · 2020-03-25T10:35:36Z

/test images

JoelSpeed

Generally looks good once the tests are passing, added a few questions as I was looking through

JoelSpeed · 2020-03-25T10:35:06Z

pkg/apis/machine/v1beta1/machinehealthcheck_types.go

@@ -99,7 +99,6 @@ type MachineHealthCheckStatus struct {
 	ExpectedMachines *int `json:"expectedMachines"`

 	// total number of machines counted by this machine health check
-	// +kubebuilder:default=0


Is this not allowed anymore?

nope, API machinery complains. Note this never applied since defaulting requires a resource to be structural schema to work.

JoelSpeed · 2020-03-25T10:36:24Z

pkg/controller/machineset/machineset_controller_test.go

 				select {
 				case recv := <-requests:
 					if recv != tc.expectedRequest {
 						t.Error("received request does not match expected request")
 					}
+
+					t.Logf("Last reconcile %v", <-requests)


Is this channel guaranteed to receive another request? This seems like an odd thing to add

Definitely odd, I couldn't really figure out where is the last request coming from :/. I'd suggest though we focus on following up to replace this completely with ginkgo like coverage (OCPCLOUD-816).

pkg/operator/operator_test.go

This commit mostly ensures we set the API version in the expected objects otherwise equality.Semantic.DeepEqual fails to compare with what is coming from the fake client.

enxebre · 2020-03-25T13:21:35Z

/test unit

JoelSpeed · 2020-03-25T13:35:33Z

/lgtm

I agree with your comments @enxebre, follow up for refactoring tests sounds good

openshift-ci-robot · 2020-03-25T14:12:52Z

@enxebre: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp	`e50e0e3`	link	`/test e2e-gcp`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

JoelSpeed · 2020-03-25T17:07:34Z

/approve

openshift-ci-robot · 2020-03-25T17:08:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed

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

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

openshift-ci-robot · 2020-03-25T17:11:15Z

@enxebre: All pull requests linked via external trackers have merged. Bugzilla bug 1816624 has been moved to the MODIFIED state.

In response to this:

BUG 1816624: MHC structural schema

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.

markmc · 2020-03-28T16:26:54Z

pkg/operator/sync.go

+		}
+	}
+	if d != nil {
+		generation = d.Status.ObservedGeneration


This looks broken, see #541

enxebre added 6 commits March 24, 2020 19:18

Regenerate clients and informers

17826db

Drop cvo as a library in favour of library-go to satisfy kube 1.18 deps

69986ee

Update pkg/operator to satisfy kube 1.18 deps

c3d3bb8

Update to use promhttp.Handler() as prometheus.Handler() was dropped …

cc8116d

…support after adding kube 1.18 deps

Update machine controller to satisfy kube 1.18 deps

e7b4c23

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Mar 24, 2020

openshift-ci-robot requested review from JoelSpeed and michaelgugino March 24, 2020 18:30

enxebre mentioned this pull request Mar 24, 2020

Update mao jobs to go 1.13 openshift/release#7889

Merged

enxebre force-pushed the mhc-structural-schema branch 4 times, most recently from a407416 to 015378d Compare March 25, 2020 00:03

enxebre added 2 commits March 25, 2020 11:41

Update makefile and scripts to use go 1.13 to satisfy kube 1.18 deps

9b58462

Regenerate CRDs and add preserveUnknownFields:false for MHC

8d16ab9

JoelSpeed reviewed Mar 25, 2020

View reviewed changes

enxebre force-pushed the mhc-structural-schema branch from 015378d to 8f75434 Compare March 25, 2020 10:46

Get unit tests green after kube 1.18 deps update

e50e0e3

This commit mostly ensures we set the API version in the expected objects otherwise equality.Semantic.DeepEqual fails to compare with what is coming from the fake client.

enxebre force-pushed the mhc-structural-schema branch from 8f75434 to e50e0e3 Compare March 25, 2020 12:51

openshift-ci-robot assigned JoelSpeed Mar 25, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 25, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 25, 2020

openshift-merge-robot merged commit 9320438 into openshift:master Mar 25, 2020

enxebre mentioned this pull request Mar 26, 2020

[OCPCLOUD-803] Run Spot Termination Handler from Machine API Operator #535

Merged

markmc reviewed Mar 28, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG 1816624: MHC structural schema #536

BUG 1816624: MHC structural schema #536

enxebre commented Mar 24, 2020 •

edited

openshift-ci-robot commented Mar 24, 2020

openshift-ci-robot commented Mar 24, 2020

enxebre commented Mar 24, 2020

openshift-ci-robot commented Mar 24, 2020

enxebre commented Mar 25, 2020

enxebre commented Mar 25, 2020

enxebre commented Mar 25, 2020

JoelSpeed left a comment

JoelSpeed Mar 25, 2020

enxebre Mar 25, 2020 •

edited

JoelSpeed Mar 25, 2020

enxebre Mar 25, 2020

enxebre commented Mar 25, 2020

JoelSpeed commented Mar 25, 2020

openshift-ci-robot commented Mar 25, 2020 •

edited

JoelSpeed commented Mar 25, 2020

openshift-ci-robot commented Mar 25, 2020

openshift-ci-robot commented Mar 25, 2020

markmc Mar 28, 2020

BUG 1816624: MHC structural schema #536

BUG 1816624: MHC structural schema #536

Conversation

enxebre commented Mar 24, 2020 • edited

openshift-ci-robot commented Mar 24, 2020

openshift-ci-robot commented Mar 24, 2020

enxebre commented Mar 24, 2020

openshift-ci-robot commented Mar 24, 2020

enxebre commented Mar 25, 2020

enxebre commented Mar 25, 2020

enxebre commented Mar 25, 2020

JoelSpeed left a comment

Choose a reason for hiding this comment

JoelSpeed Mar 25, 2020

Choose a reason for hiding this comment

enxebre Mar 25, 2020 • edited

Choose a reason for hiding this comment

JoelSpeed Mar 25, 2020

Choose a reason for hiding this comment

enxebre Mar 25, 2020

Choose a reason for hiding this comment

enxebre commented Mar 25, 2020

JoelSpeed commented Mar 25, 2020

openshift-ci-robot commented Mar 25, 2020 • edited

JoelSpeed commented Mar 25, 2020

openshift-ci-robot commented Mar 25, 2020

openshift-ci-robot commented Mar 25, 2020

markmc Mar 28, 2020

Choose a reason for hiding this comment

enxebre commented Mar 24, 2020 •

edited

enxebre Mar 25, 2020 •

edited

openshift-ci-robot commented Mar 25, 2020 •

edited