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

🌱 Remove v1beta1 type and switch v1alpha3 to use v1beta2 #4623

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,8 @@ generate-go-conversions-cabpk: $(CONVERSION_GEN)
--extra-peer-dirs=sigs.k8s.io/cluster-api/api/v1alpha3 \
--output-file-base=zz_generated.conversion $(CONVERSION_GEN_OUTPUT_BASE) \
--go-header-file=./hack/boilerplate/boilerplate.generatego.txt
$(MAKE) clean-generated-conversions SRC_DIRS="./bootstrap/kubeadm/types/v1beta1,./bootstrap/kubeadm/types/v1beta2"
$(MAKE) clean-generated-conversions SRC_DIRS="./bootstrap/kubeadm/types/v1beta2"
$(CONVERSION_GEN) \
--input-dirs=./bootstrap/kubeadm/types/v1beta1 \
--input-dirs=./bootstrap/kubeadm/types/v1beta2 \
--build-tag=ignore_autogenerated_kubeadm_bootstrap_v1alpha3 \
--output-file-base=zz_generated.conversion $(CONVERSION_GEN_OUTPUT_BASE) \
Expand Down
27 changes: 22 additions & 5 deletions bootstrap/kubeadm/api/v1alpha3/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package v1alpha3
import (
apiconversion "k8s.io/apimachinery/pkg/conversion"
kubeadmbootstrapv1alpha4 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4"
kubeadmbootstrapv1beta1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
kubeadmbootstrapv1beta2 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta2"
"sigs.k8s.io/controller-runtime/pkg/conversion"
)

Expand Down Expand Up @@ -77,16 +77,33 @@ func Convert_v1alpha3_KubeadmConfigStatus_To_v1alpha4_KubeadmConfigStatus(in *Ku
return autoConvert_v1alpha3_KubeadmConfigStatus_To_v1alpha4_KubeadmConfigStatus(in, out, s)
}

func Convert_v1alpha4_ClusterConfiguration_To_v1beta1_ClusterConfiguration(in *kubeadmbootstrapv1alpha4.ClusterConfiguration, out *kubeadmbootstrapv1beta1.ClusterConfiguration, s apiconversion.Scope) error {
func Convert_v1alpha4_ClusterConfiguration_To_v1beta2_ClusterConfiguration(in *kubeadmbootstrapv1alpha4.ClusterConfiguration, out *kubeadmbootstrapv1beta2.ClusterConfiguration, s apiconversion.Scope) error {
// DNS.Type was removed in v1alpha4 because only CoreDNS is supported; the information will be left to empty (kubeadm defaults it to CoredDNS);
// Existing clusters using kube-dns or other DNS solutions will continue to be managed/supported via the skip-coredns annotation.

// ClusterConfiguration.UseHyperKubeImage was removed in kubeadm v1alpha4 API
return kubeadmbootstrapv1beta1.Convert_v1alpha4_ClusterConfiguration_To_v1beta1_ClusterConfiguration(in, out, s)
return kubeadmbootstrapv1beta2.Convert_v1alpha4_ClusterConfiguration_To_v1beta2_ClusterConfiguration(in, out, s)
}

func Convert_v1beta1_ClusterConfiguration_To_v1alpha4_ClusterConfiguration(in *kubeadmbootstrapv1beta1.ClusterConfiguration, out *kubeadmbootstrapv1alpha4.ClusterConfiguration, s apiconversion.Scope) error {
func Convert_v1beta2_ClusterConfiguration_To_v1alpha4_ClusterConfiguration(in *kubeadmbootstrapv1beta2.ClusterConfiguration, out *kubeadmbootstrapv1alpha4.ClusterConfiguration, s apiconversion.Scope) error {
// DNS.Type was removed in v1alpha4 because only CoreDNS is supported; the information will be left to empty (kubeadm defaults it to CoredDNS);
// ClusterConfiguration.UseHyperKubeImage was removed in kubeadm v1alpha4 API
return kubeadmbootstrapv1beta1.Convert_v1beta1_ClusterConfiguration_To_v1alpha4_ClusterConfiguration(in, out, s)
return kubeadmbootstrapv1beta2.Convert_v1beta2_ClusterConfiguration_To_v1alpha4_ClusterConfiguration(in, out, s)
}

func Convert_v1alpha4_InitConfiguration_To_v1beta2_InitConfiguration(in *kubeadmbootstrapv1alpha4.InitConfiguration, out *kubeadmbootstrapv1beta2.InitConfiguration, s apiconversion.Scope) error {
return kubeadmbootstrapv1beta2.Convert_v1alpha4_InitConfiguration_To_v1beta2_InitConfiguration(in, out, s)
}

func Convert_v1beta2_InitConfiguration_To_v1alpha4_InitConfiguration(in *kubeadmbootstrapv1beta2.InitConfiguration, out *kubeadmbootstrapv1alpha4.InitConfiguration, s apiconversion.Scope) error {
// InitConfiguration.CertificateKey exists in v1beta2 types but not in bootstrapv1.InitConfiguration (Cluster API does not uses automatic copy certs). Ignoring when converting.
return kubeadmbootstrapv1beta2.Convert_v1beta2_InitConfiguration_To_v1alpha4_InitConfiguration(in, out, s)
}

func Convert_v1beta2_JoinConfiguration_To_v1alpha4_JoinConfiguration(in *kubeadmbootstrapv1beta2.JoinConfiguration, out *kubeadmbootstrapv1alpha4.JoinConfiguration, s apiconversion.Scope) error {
return kubeadmbootstrapv1beta2.Convert_v1beta2_JoinConfiguration_To_v1alpha4_JoinConfiguration(in, out, s)
}

func Convert_v1alpha4_JoinConfiguration_To_v1beta2_JoinConfiguration(in *kubeadmbootstrapv1alpha4.JoinConfiguration, out *kubeadmbootstrapv1beta2.JoinConfiguration, s apiconversion.Scope) error {
return kubeadmbootstrapv1beta2.Convert_v1alpha4_JoinConfiguration_To_v1beta2_JoinConfiguration(in, out, s)
}
34 changes: 30 additions & 4 deletions bootstrap/kubeadm/api/v1alpha3/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (

"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4"
"sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
"sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta2"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
)

Expand Down Expand Up @@ -54,7 +54,10 @@ func fuzzFuncs(_ runtimeserializer.CodecFactory) []interface{} {
return []interface{}{
KubeadmConfigStatusFuzzer,
dnsFuzzer,
initConfigFuzzer,
joinConfigFuzzer,
clusterConfigurationFuzzer,
nodeRegistrationOptionsFuzzer,
}
}

Expand All @@ -65,16 +68,39 @@ func KubeadmConfigStatusFuzzer(obj *KubeadmConfigStatus, c fuzz.Continue) {
obj.BootstrapData = nil
}

func dnsFuzzer(obj *v1beta1.DNS, c fuzz.Continue) {
func dnsFuzzer(obj *v1beta2.DNS, c fuzz.Continue) {
c.FuzzNoCustom(obj)

// DNS.Type does not exists in v1alpha4, so setting it to empty string in order to avoid v1alpha3 --> v1alpha4 --> v1alpha3 round trip errors.
obj.Type = ""
}

func clusterConfigurationFuzzer(obj *v1beta1.ClusterConfiguration, c fuzz.Continue) {
func initConfigFuzzer(obj *v1beta2.InitConfiguration, c fuzz.Continue) {
c.FuzzNoCustom(obj)

// ClusterConfiguration.UseHyperKubeImage has been removed in v1alpha4, so setting it to false in order to avoid v1beta1 --> v1alpha4 --> v1beta1 round trip errors.
// InitConfiguration.CertificateKey does not exists in v1alpha4, so setting it to empty string in order to avoid v1alpha3 --> v1alpha4 --> v1alpha3 round trip errors.
obj.CertificateKey = ""
}

func joinConfigFuzzer(obj *v1beta2.JoinConfiguration, c fuzz.Continue) {
c.FuzzNoCustom(obj)

// JoinConfiguration.ControlPlane.CertificateKey does not exists in v1alpha4, so setting it to empty string in order to avoid v1alpha3 --> v1alpha4 --> v1alpha3 round trip errors.
if obj.ControlPlane != nil {
obj.ControlPlane.CertificateKey = ""
}
}

func nodeRegistrationOptionsFuzzer(obj *v1beta2.NodeRegistrationOptions, c fuzz.Continue) {
c.FuzzNoCustom(obj)

// NodeRegistrationOptions.IgnorePreflightErrors does not exists in v1alpha4, so setting it to nil in order to avoid v1alpha3 --> v1alpha4 --> v1alpha3 round trip errors.
obj.IgnorePreflightErrors = nil
}

func clusterConfigurationFuzzer(obj *v1beta2.ClusterConfiguration, c fuzz.Continue) {
c.FuzzNoCustom(obj)

// ClusterConfiguration.UseHyperKubeImage has been removed in v1alpha4, so setting it to false in order to avoid v1alpha3 --> v1alpha4 --> v1alpha3 round trip errors.
obj.UseHyperKubeImage = false
}
8 changes: 4 additions & 4 deletions bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package v1alpha3
import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
kubeadmv1beta1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
kubeadmv1beta2 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gab-satchi
I feel really bad about this because I opened the issue behind this PR, but now that I see that removing v1beta1 is impacting v1alpha3 I don't think we can move on with this change, otherwise we can't ensure a clean v1alpha3 --> v1alpha4 upgrade.
This change has to wait for v1alpha3 being removed before 😞
Sorry again for not having considered this when opening the issue
/hold

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably mostly drop support / deprecate in this release, rather than removing the APIs altogether. In the next release, we should be able to proceed with the complete removal, alongside the v1alpha3 API types.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries @fabriziopandini. let me know if it makes more sense to close and re-open when the time is right

)

// Format specifies the output format of the bootstrap data
Expand All @@ -36,15 +36,15 @@ const (
type KubeadmConfigSpec struct {
// ClusterConfiguration along with InitConfiguration are the configurations necessary for the init command
// +optional
ClusterConfiguration *kubeadmv1beta1.ClusterConfiguration `json:"clusterConfiguration,omitempty"`
ClusterConfiguration *kubeadmv1beta2.ClusterConfiguration `json:"clusterConfiguration,omitempty"`

// InitConfiguration along with ClusterConfiguration are the configurations necessary for the init command
// +optional
InitConfiguration *kubeadmv1beta1.InitConfiguration `json:"initConfiguration,omitempty"`
InitConfiguration *kubeadmv1beta2.InitConfiguration `json:"initConfiguration,omitempty"`

// JoinConfiguration is the kubeadm configuration for the join command
// +optional
JoinConfiguration *kubeadmv1beta1.JoinConfiguration `json:"joinConfiguration,omitempty"`
JoinConfiguration *kubeadmv1beta2.JoinConfiguration `json:"joinConfiguration,omitempty"`

// Files specifies extra files to be passed to user_data upon creation.
// +optional
Expand Down
18 changes: 9 additions & 9 deletions bootstrap/kubeadm/api/v1alpha3/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"

kubeadmv1beta1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
kubeadmv1beta2 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta2"
"sigs.k8s.io/cluster-api/util"
)

Expand Down Expand Up @@ -73,14 +73,14 @@ func TestKubeadmConfigTemplateConversion(t *testing.T) {
}

var fakeKubeadmConfigSpec = KubeadmConfigSpec{
ClusterConfiguration: &kubeadmv1beta1.ClusterConfiguration{
ClusterConfiguration: &kubeadmv1beta2.ClusterConfiguration{
KubernetesVersion: "v1.20.2",
APIServer: kubeadmv1beta1.APIServer{
ControlPlaneComponent: kubeadmv1beta1.ControlPlaneComponent{
APIServer: kubeadmv1beta2.APIServer{
ControlPlaneComponent: kubeadmv1beta2.ControlPlaneComponent{
ExtraArgs: map[string]string{
"foo": "bar",
},
ExtraVolumes: []kubeadmv1beta1.HostPathMount{
ExtraVolumes: []kubeadmv1beta2.HostPathMount{
{
Name: "mount-path",
HostPath: "/foo",
Expand All @@ -91,14 +91,14 @@ var fakeKubeadmConfigSpec = KubeadmConfigSpec{
},
},
},
InitConfiguration: &kubeadmv1beta1.InitConfiguration{
NodeRegistration: kubeadmv1beta1.NodeRegistrationOptions{
InitConfiguration: &kubeadmv1beta2.InitConfiguration{
NodeRegistration: kubeadmv1beta2.NodeRegistrationOptions{
Name: "foo",
CRISocket: "/var/run/containerd/containerd.sock",
},
},
JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{
NodeRegistration: kubeadmv1beta1.NodeRegistrationOptions{
JoinConfiguration: &kubeadmv1beta2.JoinConfiguration{
NodeRegistration: kubeadmv1beta2.NodeRegistrationOptions{
Name: "foo",
CRISocket: "/var/run/containerd/containerd.sock",
},
Expand Down
76 changes: 64 additions & 12 deletions bootstrap/kubeadm/api/v1alpha3/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading