Add v1alpha2 types 🎉 #1051

vincepri · 2019-06-20T04:05:10Z

Signed-off-by: Vince Prignano [email protected]

What this PR does / why we need it:

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 #1035
Fixes #1073

Special notes for your reviewer:

Ongoing list of TODOs:

Decide what to do with Cluster, are we porting it with providerSpec or leave it on v1alpha1?
Regenerate clientset targeting v1alpha2
Move, copy, and adapt tests from v1alpha1 types

Release note:

k8s-ci-robot · 2019-06-20T04:05:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vincepri

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

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

pablochacin · 2019-06-20T22:18:59Z

Decide what to do with Cluster, are we porting it with providerSpec or leave it on v1alpha1?

@vincepri Can you please elaborate on this? I'm not sure what "porting it with providerSpec" means. Something related to #833 (comment)

vincepri · 2019-06-20T22:28:41Z

@pablochacin Cluster will be ported to v1alpha2 as is from v1alpha1 (in this PR). Whenever the proposal is ready and accepted, we can modify the v1alpha2 type to match.

vincepri · 2019-06-24T23:42:49Z

/test pull-cluster-api-test

vincepri · 2019-06-25T18:34:15Z

/check-cla

vincepri · 2019-06-25T18:34:31Z

/assign @ncdc @detiber

vincepri · 2019-06-25T21:33:59Z

@ncdc Running informer-gen separately with some debug logs shows a weird behavior when multiple folders that aren't in the same APIGroup, are passed as input-dirs.

--input-dirs sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1

...
(map[string]types.GroupVersions) (len=1) {
 (string) (len=7) "cluster": (types.GroupVersions) {
  PackageName: (string) (len=7) "cluster",
  Group: (types.Group) (len=14) cluster.k8s.io,
  Versions: ([]types.PackageVersion) (len=1 cap=1) {
   (types.PackageVersion) v1alpha1
  }
 }
}
...

--input-dirs sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha2

(map[string]types.GroupVersions) (len=1) {
 (string) (len=7) "cluster": (types.GroupVersions) {
  PackageName: (string) (len=7) "cluster",
  Group: (types.Group) (len=19) cluster.sigs.k8s.io,
  Versions: ([]types.PackageVersion) (len=1 cap=1) {
   (types.PackageVersion) v1alpha2
  }
 }
}

--input-dirs sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1,sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha2

(map[string]types.GroupVersions) (len=1) {
 (string) (len=7) "cluster": (types.GroupVersions) {
  PackageName: (string) (len=7) "cluster",
  Group: (types.Group) (len=14) cluster.k8s.io,
  Versions: ([]types.PackageVersion) (len=2 cap=2) {
   (types.PackageVersion) v1alpha1,
   (types.PackageVersion) v1alpha2
  }
 }
}
panic: runtime error: index out of range

goroutine 1 [running]:
k8s.io/code-generator/cmd/informer-gen/generators.(*genericGenerator).GenerateType.func1(...)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/code-generator/cmd/informer-gen/generators/generic.go:120
k8s.io/code-generator/cmd/informer-gen/generators.(*genericGenerator).GenerateType(0xc003d82690, 0xc00a6d5440, 0xc00a391810, 0x142bee0, 0xc00a6ed8e0, 0x0, 0x139e7f8)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/code-generator/cmd/informer-gen/generators/generic.go:121 +0x1060
k8s.io/gengo/generator.(*Context).executeBody(0xc00a6d5440, 0x142bae0, 0xc00a708240, 0x14377a0, 0xc003d82690, 0x480, 0x0)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/gengo/generator/execute.go:304 +0x11d
k8s.io/gengo/generator.(*Context).ExecutePackage(0xc00a3b4240, 0xc0001ae140, 0x13, 0x1434aa0, 0xc00a425100, 0x0, 0x0)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/gengo/generator/execute.go:265 +0xbf7
k8s.io/gengo/generator.(*Context).ExecutePackages(0xc00a3b4240, 0xc0001ae140, 0x13, 0xc00a425200, 0x5, 0x8, 0x0, 0x12e1136)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/gengo/generator/execute.go:51 +0xc5
k8s.io/gengo/args.(*GeneratorArgs).Execute(0xc0001b41e0, 0xc0001b23f0, 0x139d3e0, 0x6, 0x13ccb70, 0x0, 0x0)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/gengo/args/args.go:194 +0x2d7
main.main()
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/code-generator/cmd/informer-gen/main.go:55 +0x320
exit status 2

--input-dirs sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha2,sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1

(map[string]types.GroupVersions) (len=1) {
 (string) (len=7) "cluster": (types.GroupVersions) {
  PackageName: (string) (len=7) "cluster",
  Group: (types.Group) (len=19) cluster.sigs.k8s.io,
  Versions: ([]types.PackageVersion) (len=2 cap=2) {
   (types.PackageVersion) v1alpha2,
   (types.PackageVersion) v1alpha1
  }
 }
}
panic: runtime error: index out of range

goroutine 1 [running]:
k8s.io/code-generator/cmd/informer-gen/generators.(*genericGenerator).GenerateType.func1(...)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/code-generator/cmd/informer-gen/generators/generic.go:120
k8s.io/code-generator/cmd/informer-gen/generators.(*genericGenerator).GenerateType(0xc0016167e0, 0xc00a58ec00, 0xc00a354d10, 0x142bee0, 0xc00a577580, 0x0, 0x139e7f8)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/code-generator/cmd/informer-gen/generators/generic.go:121 +0x1060
k8s.io/gengo/generator.(*Context).executeBody(0xc00a58ec00, 0x142bae0, 0xc00a56d4f0, 0x14377a0, 0xc0016167e0, 0x480, 0x0)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/gengo/generator/execute.go:304 +0x11d
k8s.io/gengo/generator.(*Context).ExecutePackage(0xc00a30d800, 0xc000016380, 0x13, 0x1434aa0, 0xc00a3e6600, 0x0, 0x0)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/gengo/generator/execute.go:265 +0xbf7
k8s.io/gengo/generator.(*Context).ExecutePackages(0xc00a30d800, 0xc000016380, 0x13, 0xc00a3e6700, 0x5, 0x8, 0x0, 0x12e1136)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/gengo/generator/execute.go:51 +0xc5
k8s.io/gengo/args.(*GeneratorArgs).Execute(0xc000012280, 0xc0000ce570, 0x139d3e0, 0x6, 0x13ccb70, 0x0, 0x0)
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/gengo/args/args.go:194 +0x2d7
main.main()
        /Users/vince/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/code-generator/cmd/informer-gen/main.go:55 +0x320
exit status 2

Makefile

pkg/apis/cluster/v1alpha2/cluster_types.go

pkg/apis/cluster/v1alpha2/cluster_types_test.go

pkg/apis/cluster/v1alpha2/common_type.go

pkg/apis/cluster/v1alpha2/machine_types.go

pkg/controller/cluster/cluster_controller.go

scripts/ci-integration.sh

test/integration/cluster/cluster_test.go

ncdc · 2019-06-26T14:23:02Z

@vincepri informer-gen requires that all API versions in pkg/apis/$foo share the same group. Let's move pkg/apis/cluster/v1alpha1 to something like pkg/apis/deprecated/v1alpha1 and keep pkg/apis/cluster/v1alpha2. Then informer-gen will work.

scripts/ci-integration.sh

vincepri · 2019-06-26T16:00:50Z

@ncdc I actually removed the kind file and the feature gates now that we're using trivialVersions for all the CRDs

vincepri · 2019-06-26T17:14:02Z

/test pull-cluster-api-test

vincepri · 2019-06-26T17:18:48Z

/test pull-cluster-api-test

vincepri · 2019-06-26T17:32:51Z

/test pull-cluster-api-test

Signed-off-by: Vince Prignano <[email protected]>

ncdc · 2019-06-26T17:49:10Z

/lgtm

detiber · 2019-06-26T19:51:07Z

cmd/clusterctl/clusterdeployer/clusterclient/clusterclient.go

-				klog.V(10).Info("retrying: machine.Status.ProviderStatus is not set")
-				return false, nil
-			}
+			// if m.Status.ProviderStatus == nil {


Why commented out here, should this have been replaced with a TODO instead?

detiber · 2019-06-26T20:00:07Z

config/crds/cluster.k8s.io_clusters.yaml

@@ -13,8 +13,6 @@ spec:
    shortNames:
    - cl
  scope: Namespaced
-  subresources:
-    status: {}


What happened to the status subresource?

detiber · 2019-06-26T20:00:17Z

config/crds/cluster.k8s.io_machinedeployments.yaml

@@ -18,7 +18,6 @@ spec:
      labelSelectorPath: .status.labelSelector
      specReplicasPath: .spec.replicas
      statusReplicasPath: .status.replicas
-    status: {}


What happened to the status subresource?

Good point, this needs to be added back. It was conflicting when both v1alpha2 and v1alpha2 were in the same CRD

detiber · 2019-06-26T20:00:24Z

config/crds/cluster.k8s.io_machines.yaml

  group: cluster.k8s.io
  names:
    kind: Machine
    plural: machines
    shortNames:
    - ma
  scope: Namespaced
-  subresources:
-    status: {}


What happened to the status subresource?

Same as above

detiber · 2019-06-26T20:00:31Z

config/crds/cluster.k8s.io_machinesets.yaml

@@ -18,7 +18,6 @@ spec:
      labelSelectorPath: .status.labelSelector
      specReplicasPath: .spec.replicas
      statusReplicasPath: .status.replicas
-    status: {}


What happened to the status subresource?

Same as above

detiber · 2019-06-26T20:05:25Z

config/samples/cluster_v1alpha2_cluster.yaml

@@ -0,0 +1,9 @@
+apiVersion: cluster.k8s.io/v1alpha2


Same thing with the v1alpha2 samples, they are invalid and can likely just be removed.

detiber · 2019-06-26T20:13:48Z

pkg/apis/cluster/v1alpha2/machine_types.go

+/// [MachineSpec]
+// MachineSpec defines the desired state of Machine
+type MachineSpec struct {
+	// ObjectMeta will autopopulate the Node created. Use this to


Do we actually expect this to affect the created Node? That seems more like a concern for the bootstrap controller than something explicitly on the MachineSpec.

I'm not sure I'm completely convinced here that we need an embedded ObjectMeta on the MachineSpec type.

Ah, I see why this was confusing now... ObjectMeta should not be needed on the MachineSpec, only on the MachineTemplateSpec embedded type:

See PodTemplateSpec and PodSpec for comparison

detiber · 2019-06-26T20:26:33Z

pkg/apis/deprecated/v1alpha1/machine_types.go

@@ -38,10 +38,6 @@ const (
 // Machine is the Schema for the machines API
 // +k8s:openapi-gen=true
 // +kubebuilder:resource:path=machines,shortName=ma
-// +kubebuilder:subresource:status


The subresource status should probably remain.

detiber · 2019-06-26T20:26:48Z

pkg/apis/deprecated/v1alpha1/machinedeployment_types.go

@@ -168,7 +168,6 @@ type MachineDeploymentStatus struct {
 // MachineDeployment is the Schema for the machinedeployments API
 // +k8s:openapi-gen=true
 // +kubebuilder:resource:path=machinedeployments,shortName=md
-// +kubebuilder:subresource:status


The subresource status should probably remain.

detiber · 2019-06-26T20:26:53Z

pkg/apis/deprecated/v1alpha1/machineset_types.go

@@ -33,7 +33,6 @@ import (
 // MachineSet ensures that a specified number of machines replicas are running at any given time.
 // +k8s:openapi-gen=true
 // +kubebuilder:resource:path=machinesets,shortName=ms
-// +kubebuilder:subresource:status


The subresource status should probably remain.

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 20, 2019

k8s-ci-robot requested review from detiber and justinsb June 20, 2019 04:05

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 20, 2019

vincepri force-pushed the v1alpha2-types branch from 1c89c00 to c16e6a8 Compare June 20, 2019 04:08

vincepri force-pushed the v1alpha2-types branch 2 times, most recently from 169bb0f to 181c713 Compare June 20, 2019 19:38

vincepri force-pushed the v1alpha2-types branch 2 times, most recently from 117665f to c9bb768 Compare June 24, 2019 23:26

vincepri force-pushed the v1alpha2-types branch from c9bb768 to 631f07c Compare June 24, 2019 23:48

vincepri changed the title ~~WIP: Add v1alpha2 types 🎉~~ Add v1alpha2 types 🎉 Jun 24, 2019

k8s-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 24, 2019

vincepri mentioned this pull request Jun 25, 2019

Add noderef controller documentation to gitbook #1066

Merged

vincepri force-pushed the v1alpha2-types branch from f4be9c2 to ae52102 Compare June 25, 2019 16:07

k8s-ci-robot assigned detiber and ncdc Jun 25, 2019

ncdc reviewed Jun 26, 2019

View reviewed changes

vincepri force-pushed the v1alpha2-types branch from 7b3ef39 to d88d6af Compare June 26, 2019 15:16

ncdc reviewed Jun 26, 2019

View reviewed changes

scripts/ci-integration.sh Outdated Show resolved Hide resolved

ncdc reviewed Jun 26, 2019

View reviewed changes

scripts/ci-integration.sh Outdated Show resolved Hide resolved

vincepri force-pushed the v1alpha2-types branch 4 times, most recently from 86fc928 to c851f78 Compare June 26, 2019 15:57

vincepri force-pushed the v1alpha2-types branch 2 times, most recently from d092ed0 to 43ad90c Compare June 26, 2019 16:47

vincepri force-pushed the v1alpha2-types branch from ad6509a to 2bb2e91 Compare June 26, 2019 17:24

Add v1alpha2 types 🎉

9bb066f

Signed-off-by: Vince Prignano <[email protected]>

vincepri force-pushed the v1alpha2-types branch from 2bb2e91 to 9bb066f Compare June 26, 2019 17:42

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 26, 2019

k8s-ci-robot merged commit 377ad4e into kubernetes-sigs:master Jun 26, 2019

vincepri mentioned this pull request Jun 26, 2019

Deprecate node controller #1041

Closed

detiber reviewed Jun 26, 2019

View reviewed changes

vincepri mentioned this pull request Jul 12, 2019

Rename controller name with - instead of _ #1149

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add v1alpha2 types 🎉 #1051

Add v1alpha2 types 🎉 #1051

vincepri commented Jun 20, 2019 •

edited

Loading

k8s-ci-robot commented Jun 20, 2019

pablochacin commented Jun 20, 2019 •

edited

Loading

vincepri commented Jun 20, 2019 •

edited

Loading

vincepri commented Jun 24, 2019

vincepri commented Jun 25, 2019

vincepri commented Jun 25, 2019

vincepri commented Jun 25, 2019

ncdc commented Jun 26, 2019

vincepri commented Jun 26, 2019

vincepri commented Jun 26, 2019

vincepri commented Jun 26, 2019

vincepri commented Jun 26, 2019

ncdc commented Jun 26, 2019

detiber Jun 26, 2019

detiber Jun 26, 2019

detiber Jun 26, 2019

vincepri Jun 26, 2019

detiber Jun 26, 2019

vincepri Jun 26, 2019

detiber Jun 26, 2019

vincepri Jun 26, 2019

detiber Jun 26, 2019

detiber Jun 26, 2019

detiber Jun 26, 2019

detiber Jun 26, 2019

detiber Jun 26, 2019

detiber Jun 26, 2019

Add v1alpha2 types 🎉 #1051

Add v1alpha2 types 🎉 #1051

Conversation

vincepri commented Jun 20, 2019 • edited Loading

k8s-ci-robot commented Jun 20, 2019

pablochacin commented Jun 20, 2019 • edited Loading

vincepri commented Jun 20, 2019 • edited Loading

vincepri commented Jun 24, 2019

vincepri commented Jun 25, 2019

vincepri commented Jun 25, 2019

vincepri commented Jun 25, 2019

--input-dirs sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1

--input-dirs sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha2

--input-dirs sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1,sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha2

--input-dirs sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha2,sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1

ncdc commented Jun 26, 2019

vincepri commented Jun 26, 2019

vincepri commented Jun 26, 2019

vincepri commented Jun 26, 2019

vincepri commented Jun 26, 2019

ncdc commented Jun 26, 2019

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vincepri commented Jun 20, 2019 •

edited

Loading

pablochacin commented Jun 20, 2019 •

edited

Loading

vincepri commented Jun 20, 2019 •

edited

Loading