Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade the controller-runtime version to v0.8.x #4096

Closed
srm09 opened this issue Jan 20, 2021 · 7 comments · Fixed by #4109
Closed

Upgrade the controller-runtime version to v0.8.x #4096

srm09 opened this issue Jan 20, 2021 · 7 comments · Fixed by #4109
Assignees
Labels
area/dependency Issues or PRs related to dependency changes kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@srm09
Copy link
Contributor

srm09 commented Jan 20, 2021

User Story
controller-runtime is a dependency that is vendored in the CAPI codebase. Recently, a newer version of controller-runtime has been released.

Detailed Description
n/a

Anything else you would like to add:
The k8s bits in the controller-runtime packages have been updated to v1.20.1 . But the minimum supported version for the management cluster should still remain at v1.19.0 in the test cases.

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 20, 2021
@srm09
Copy link
Contributor Author

srm09 commented Jan 20, 2021

/assign

@vincepri
Copy link
Member

/milestone v0.4.0
/priority important-soon

@k8s-ci-robot k8s-ci-robot added this to the v0.4.0 milestone Jan 21, 2021
@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jan 21, 2021
@vincepri
Copy link
Member

/area dependency

@k8s-ci-robot k8s-ci-robot added the area/dependency Issues or PRs related to dependency changes label Jan 21, 2021
@srm09
Copy link
Contributor Author

srm09 commented Jan 22, 2021

FYI, this update causes conversion-gen to incorrectly generate conversion functions for types that have slices of pointers to custom objects.
For example: checkout DockerMachinePoolStatus type

This is the error snippet from the generated conversion file:

func autoConvert_v1alpha4_DockerMachinePoolStatus_To_v1alpha3_DockerMachinePoolStatus(in *v1alpha4.DockerMachinePoolStatus, out *DockerMachinePoolStatus, s conversion.Scope) error {
	out.Ready = in.Ready
	out.Replicas = in.Replicas
	out.ObservedGeneration = in.ObservedGeneration
	if in.Instances != nil {
		in, out := &in.Instances, &out.Instances
		*out = make([]*DockerMachinePoolInstanceStatus, len(*in))
		for i := range *in {
			// FIXME: Provide conversion function to convert *v1alpha4.DockerMachinePoolInstanceStatus to *DockerMachinePoolInstanceStatus 
                        // srm09: THE ERROR IS HERE ^
			compileErrorOnMissingConversion()
		}
	} else {
		out.Instances = nil
	}
	if in.Conditions != nil {
		in, out := &in.Conditions, &out.Conditions
		*out = make(apiv1alpha3.Conditions, len(*in))
		for i := range *in {
			if err := apiv1alpha3.Convert_v1alpha4_Condition_To_v1alpha3_Condition(&(*in)[i], &(*out)[i], s); err != nil {
				return err
			}
		}
	} else {
		out.Conditions = nil
	}
	return nil
}

There is a similar error in the autoConvert function for v1alpha4.DockerMachinPoolStatus to v1alpha3.DockerMachinPoolStatus

@prankul88
Copy link
Contributor

prankul88 commented Feb 3, 2021

@srm09 Hi, does this change require to have min go version as well?

With this change merged, I got an error while running command make clusterctl

#sigs.k8s.io/controller-runtime/pkg/manager
../../../pkg/mod/sigs.k8s.io/[email protected]/pkg/manager/manager.go:53:2: duplicate method Start
note: module requires Go 1.15
Makefile:157: recipe for target 'clusterctl' failed

Edit: I was using go v1.13 and is working fine with go v1.15.7

@vincepri
Copy link
Member

vincepri commented Feb 3, 2021

@prankul88 Go 1.15 is required for v1alpha4

@prankul88
Copy link
Contributor

@vincepri Got it. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependency Issues or PRs related to dependency changes kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants