Bug 1820474: clusterAutoscaler and machineAutoscaler structural schema #146

enxebre · 2020-04-03T07:33:05Z

This makes clusterAutoscaler and machineAutoscaler structural schemas.
To this end I update controller-tools, which forced me to update the whole api machinery stack to kube 1.18. As a consequence I had to drop CVO to avoid transitive incompatible dependencies and take the chance to move to openshift/library-go. As a consequence I had to update scripts to run on golang 1.13.
Needs openshift/release#8114.

Fix client signatures Drop CVO in favour go library-go Ignore resourceVersion on spec comparisons kubernetes-sigs/controller-runtime#620

openshift-ci-robot · 2020-04-03T07:33:12Z

@enxebre: This pull request references Bugzilla bug 1820474, 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 NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1820474: clusterAutoscaler and machineAutoscaler 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-04-03T07:54:35Z

KIND:     ClusterAutoscaler
VERSION:  autoscaling.openshift.io/v1

DESCRIPTION:
     ClusterAutoscaler is the Schema for the clusterautoscalers 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>
      balanceSimilarNodeGroups	<boolean>
      ignoreDaemonsetsUtilization	<boolean>
      maxPodGracePeriod	<integer>
      podPriorityThreshold	<integer>
      resourceLimits	<Object>
         cores	<Object>
            max	<integer>
            min	<integer>
         gpus	<[]Object>
            max	<integer>
            min	<integer>
            type	<string>
         maxNodesTotal	<integer>
         memory	<Object>
            max	<integer>
            min	<integer>
      scaleDown	<Object>
         delayAfterAdd	<string>
         delayAfterDelete	<string>
         delayAfterFailure	<string>
         enabled	<boolean>
         unneededTime	<string>
      skipNodesWithLocalStorage	<boolean>
   status	<map[string]>

KIND:     MachineAutoscaler
VERSION:  autoscaling.openshift.io/v1beta1

DESCRIPTION:
     MachineAutoscaler is the Schema for the machineautoscalers 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>
      maxReplicas	<integer>
      minReplicas	<integer>
      scaleTargetRef	<Object>
         apiVersion	<string>
         kind	<string>
         name	<string>
   status	<Object>
      lastTargetRef	<Object>
         apiVersion	<string>
         kind	<string>
         name	<string>

openshift-ci-robot · 2020-04-03T08:00:23Z

@enxebre: This pull request references Bugzilla bug 1820474, 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 1820474: clusterAutoscaler and machineAutoscaler 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.

JoelSpeed

/approve

JoelSpeed · 2020-04-03T09:58:06Z

/retest

openshift-ci-robot · 2020-04-03T09:58:21Z

[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

JoelSpeed · 2020-04-03T10:09:22Z

L13 of the Makefile needs to go from GOFLAGS += -mod=vendor to GOFLAGS ?= -mod=vendor to work with Go 1.13, the flag is included by default from 1.13 onwards

enxebre · 2020-04-03T15:02:28Z

/retest

elmiko

looks good to me from a logic standpoint, i just have a question about a variable name

elmiko · 2020-04-06T17:34:39Z

pkg/controller/machineautoscaler/machineautoscaler_controller.go

-	// Fail if we didn't find any of the supported target types registered.
-	if len(r.config.SupportedTargetGVKs) < 1 {
-		return ErrNoSupportedTargets
+func getMissingGVKs(restMapper meta.RESTMapper, supportedCVKs []schema.GroupVersionKind) ([]schema.GroupVersionKind, error) {


should the second argument be named supportGVKs to make it clearer?

elmiko · 2020-04-06T17:35:37Z

pkg/controller/machineautoscaler/machineautoscaler_controller.go

+func getMissingGVKs(restMapper meta.RESTMapper, supportedCVKs []schema.GroupVersionKind) ([]schema.GroupVersionKind, error) {
+	var missingGVKs []schema.GroupVersionKind
+
+	for _, gvk := range supportedCVKs {


s/supportedCVKs/supportedGVKs/ ?

Before revendoring controller-runtime `watch` was running `start` by default which made it possible to use the returned error to discriminated the existing GVKs https://github.com/openshift/cluster-autoscaler-operator/blob/a0b7aead18c86ac9a51f3479cba0c656ed83065f/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go#L122. Without this commit, the new behaviour introduced by controller-runtime https://github.com/kubernetes-sigs/controller-runtime/blob/b6092bd383f0317f48ee396629ceb46d7dd064b4/pkg/internal/controller/controller.go#L137 is hidding the missing GVKs until the manager is started resulting in a fatal error.

enxebre · 2020-04-07T08:56:03Z

/retest

alexander-demicev

/lgtm

enxebre · 2020-04-07T10:27:20Z

/test e2e-aws

enxebre · 2020-04-07T10:29:08Z

/retest

openshift-bot · 2020-04-07T10:32:48Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-04-07T11:49:46Z

@enxebre: All pull requests linked via external trackers have merged: openshift/cluster-autoscaler-operator#146. Bugzilla bug 1820474 has been moved to the MODIFIED state.

In response to this:

Bug 1820474: clusterAutoscaler and machineAutoscaler 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 added 6 commits April 2, 2020 20:00

Revendor for kube 1.18

b70084d

Get build to pass after revendor to 1.18

8ed8fe9

Fix client signatures Drop CVO in favour go library-go Ignore resourceVersion on spec comparisons kubernetes-sigs/controller-runtime#620

Fix generate deepcopy and drop unnecessary files

e92305e

Fix annotation typo in script

3a1ffdf

Regenerate CRDs to be structural schema

51a7b89

Add preserveUnknownFields: false

c91a5ee

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

openshift-ci-robot requested review from frobware and JoelSpeed April 3, 2020 07:34

enxebre mentioned this pull request Apr 3, 2020

Bump CAO to golang 1.13 openshift/release#8114

Merged

JoelSpeed reviewed Apr 3, 2020

View reviewed changes

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

Bump to golang 1.13

0bc4a9c

enxebre force-pushed the structural-schema branch from e09cb41 to 0bc4a9c Compare April 3, 2020 11:31

enxebre force-pushed the structural-schema branch 2 times, most recently from 52daa3b to 1341f59 Compare April 6, 2020 16:56

elmiko reviewed Apr 6, 2020

View reviewed changes

enxebre force-pushed the structural-schema branch from 1341f59 to 36e2318 Compare April 6, 2020 18:21

enxebre mentioned this pull request Apr 7, 2020

[OCPCLOUD-813] Bump scripts to Go 1.13 #145

Closed

alexander-demicev approved these changes Apr 7, 2020

View reviewed changes

openshift-ci-robot assigned alexander-demicev Apr 7, 2020

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

openshift-merge-robot merged commit 3ab6dc5 into openshift:master Apr 7, 2020

enxebre mentioned this pull request Apr 7, 2020

Bug 1817860: Fix API group on related resources #147

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1820474: clusterAutoscaler and machineAutoscaler structural schema #146

Bug 1820474: clusterAutoscaler and machineAutoscaler structural schema #146

enxebre commented Apr 3, 2020 •

edited

openshift-ci-robot commented Apr 3, 2020

enxebre commented Apr 3, 2020

openshift-ci-robot commented Apr 3, 2020

JoelSpeed left a comment

JoelSpeed commented Apr 3, 2020

openshift-ci-robot commented Apr 3, 2020

JoelSpeed commented Apr 3, 2020

enxebre commented Apr 3, 2020

elmiko left a comment

elmiko Apr 6, 2020

elmiko Apr 6, 2020

enxebre commented Apr 7, 2020

alexander-demicev left a comment

enxebre commented Apr 7, 2020

enxebre commented Apr 7, 2020

openshift-bot commented Apr 7, 2020

openshift-ci-robot commented Apr 7, 2020

Bug 1820474: clusterAutoscaler and machineAutoscaler structural schema #146

Bug 1820474: clusterAutoscaler and machineAutoscaler structural schema #146

Conversation

enxebre commented Apr 3, 2020 • edited

openshift-ci-robot commented Apr 3, 2020

enxebre commented Apr 3, 2020

openshift-ci-robot commented Apr 3, 2020

JoelSpeed left a comment

Choose a reason for hiding this comment

JoelSpeed commented Apr 3, 2020

openshift-ci-robot commented Apr 3, 2020

JoelSpeed commented Apr 3, 2020

enxebre commented Apr 3, 2020

elmiko left a comment

Choose a reason for hiding this comment

elmiko Apr 6, 2020

Choose a reason for hiding this comment

elmiko Apr 6, 2020

Choose a reason for hiding this comment

enxebre commented Apr 7, 2020

alexander-demicev left a comment

Choose a reason for hiding this comment

enxebre commented Apr 7, 2020

enxebre commented Apr 7, 2020

openshift-bot commented Apr 7, 2020

openshift-ci-robot commented Apr 7, 2020

enxebre commented Apr 3, 2020 •

edited