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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

馃尡 Stop relying on GVK being set on regular typed objects #9956

Merged
merged 1 commit into from
Jan 11, 2024
Merged

馃尡 Stop relying on GVK being set on regular typed objects #9956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ func (r *KubeadmConfigReconciler) handleClusterNotInitialized(ctx context.Contex
if scope.Config.Spec.InitConfiguration == nil {
scope.Config.Spec.InitConfiguration = &bootstrapv1.InitConfiguration{
TypeMeta: metav1.TypeMeta{
APIVersion: "kubeadm.k8s.io/v1beta1",
APIVersion: "kubeadm.k8s.io/v1beta3",
Copy link
Member Author

Choose a reason for hiding this comment

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

@fabriziopandini I guess this wasn't a problem because this code was never actually used?

Copy link
Member

Choose a reason for hiding this comment

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

I need to dig into this, I will follow up ASAP (eventually we can drop this from this PR if you want to move forward)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the PR doesn't change it for the worse

Kind: "InitConfiguration",
},
}
Expand All @@ -431,7 +431,7 @@ func (r *KubeadmConfigReconciler) handleClusterNotInitialized(ctx context.Contex
if scope.Config.Spec.ClusterConfiguration == nil {
scope.Config.Spec.ClusterConfiguration = &bootstrapv1.ClusterConfiguration{
TypeMeta: metav1.TypeMeta{
APIVersion: "kubeadm.k8s.io/v1beta1",
APIVersion: "kubeadm.k8s.io/v1beta3",
Kind: "ClusterConfiguration",
},
}
Expand Down
8 changes: 4 additions & 4 deletions bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ func TestKubeadmConfigReconciler_TestSecretOwnerReferenceReconciliation(t *testi

config := newKubeadmConfig(metav1.NamespaceDefault, "cfg")
config.SetOwnerReferences(util.EnsureOwnerRef(config.GetOwnerReferences(), metav1.OwnerReference{
APIVersion: machine.APIVersion,
Kind: machine.Kind,
APIVersion: clusterv1.GroupVersion.String(),
Kind: "Machine",
Name: machine.Name,
UID: machine.UID,
}))
Expand Down Expand Up @@ -207,8 +207,8 @@ func TestKubeadmConfigReconciler_TestSecretOwnerReferenceReconciliation(t *testi

actual.SetOwnerReferences([]metav1.OwnerReference{
{
APIVersion: machine.APIVersion,
Kind: machine.Kind,
APIVersion: clusterv1.GroupVersion.String(),
Kind: "Machine",
Name: machine.Name,
UID: machine.UID,
Controller: ptr.To(true),
Expand Down
4 changes: 2 additions & 2 deletions bootstrap/kubeadm/internal/locking/control_plane_init_mutex.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ func (s *semaphore) setMetadata(cluster *clusterv1.Cluster) {
},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: cluster.APIVersion,
Kind: cluster.Kind,
APIVersion: clusterv1.GroupVersion.String(),
Kind: "Cluster",
Name: cluster.Name,
UID: cluster.UID,
},
Expand Down
4 changes: 2 additions & 2 deletions exp/addons/api/v1beta1/clusterresourcesetbinding_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ func (c *ClusterResourceSetBinding) DeleteBinding(clusterResourceSet *ClusterRes
}
}
c.OwnerReferences = removeOwnerRef(c.GetOwnerReferences(), metav1.OwnerReference{
APIVersion: clusterResourceSet.APIVersion,
Kind: clusterResourceSet.Kind,
APIVersion: GroupVersion.String(),
Kind: "ClusterResourceSet",
Name: clusterResourceSet.Name,
})
}
Expand Down
6 changes: 3 additions & 3 deletions exp/addons/internal/controllers/clusterresourceset_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ func (r *ClusterResourceSetReconciler) reconcileDelete(ctx context.Context, clus

clusterResourceSetBinding.RemoveBinding(crs)
clusterResourceSetBinding.OwnerReferences = util.RemoveOwnerRef(clusterResourceSetBinding.GetOwnerReferences(), metav1.OwnerReference{
APIVersion: crs.APIVersion,
Kind: crs.Kind,
APIVersion: addonsv1.GroupVersion.String(),
Kind: "ClusterResourceSet",
Name: crs.Name,
})

Expand Down Expand Up @@ -296,7 +296,7 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
// Ensure that the owner references are set on the ClusterResourceSetBinding.
clusterResourceSetBinding.SetOwnerReferences(util.EnsureOwnerRef(clusterResourceSetBinding.GetOwnerReferences(), metav1.OwnerReference{
APIVersion: addonsv1.GroupVersion.String(),
Kind: clusterResourceSet.Kind,
Kind: "ClusterResourceSet",
Name: clusterResourceSet.Name,
UID: clusterResourceSet.UID,
}))
Expand Down
2 changes: 1 addition & 1 deletion exp/internal/controllers/machinepool_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func (r *MachinePoolReconciler) reconcile(ctx context.Context, cluster *clusterv
// Ensure the MachinePool is owned by the Cluster it belongs to.
mp.SetOwnerReferences(util.EnsureOwnerRef(mp.GetOwnerReferences(), metav1.OwnerReference{
APIVersion: clusterv1.GroupVersion.String(),
Kind: cluster.Kind,
Kind: "Cluster",
Name: cluster.Name,
UID: cluster.UID,
}))
Expand Down
4 changes: 2 additions & 2 deletions exp/internal/controllers/machinepool_controller_noderef.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ func (r *MachinePoolReconciler) getNodeReferences(ctx context.Context, c client.
ready++
}
nodeRefs = append(nodeRefs, corev1.ObjectReference{
Kind: node.Kind,
APIVersion: node.APIVersion,
APIVersion: corev1.SchemeGroupVersion.String(),
Kind: "Node",
Name: node.Name,
UID: node.UID,
})
Expand Down
5 changes: 3 additions & 2 deletions internal/contract/references.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// Ref provide a helper struct for working with references in Unstructured objects.
Expand Down Expand Up @@ -88,7 +87,9 @@ func SetNestedRef(obj, refObj *unstructured.Unstructured, fields ...string) erro
}

// ObjToRef returns a reference to the given object.
func ObjToRef(obj client.Object) *corev1.ObjectReference {
// Note: This function only operates on Unstructured instead of client.Object
// because it is only safe to assume for Unstructured that the GVK is set.
func ObjToRef(obj *unstructured.Unstructured) *corev1.ObjectReference {
gvk := obj.GetObjectKind().GroupVersionKind()
return &corev1.ObjectReference{
Kind: gvk.Kind,
Expand Down
10 changes: 5 additions & 5 deletions internal/controllers/clusterclass/clusterclass_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func assertInfrastructureClusterTemplate(ctx context.Context, actualClusterClass
if err := env.Get(ctx, actualInfraClusterTemplateKey, actualInfraClusterTemplate); err != nil {
return err
}
if err := assertHasOwnerReference(actualInfraClusterTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
if err := assertHasOwnerReference(actualInfraClusterTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
return err
}

Expand All @@ -205,7 +205,7 @@ func assertControlPlaneTemplate(ctx context.Context, actualClusterClass *cluster
if err := env.Get(ctx, actualControlPlaneTemplateKey, actualControlPlaneTemplate); err != nil {
return err
}
if err := assertHasOwnerReference(actualControlPlaneTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
if err := assertHasOwnerReference(actualControlPlaneTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
return err
}

Expand All @@ -226,7 +226,7 @@ func assertControlPlaneTemplate(ctx context.Context, actualClusterClass *cluster
if err := env.Get(ctx, actualInfrastructureMachineTemplateKey, actualInfrastructureMachineTemplate); err != nil {
return err
}
if err := assertHasOwnerReference(actualInfrastructureMachineTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
if err := assertHasOwnerReference(actualInfrastructureMachineTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
return err
}

Expand Down Expand Up @@ -260,7 +260,7 @@ func assertMachineDeploymentClass(ctx context.Context, actualClusterClass *clust
if err := env.Get(ctx, actualInfrastructureMachineTemplateKey, actualInfrastructureMachineTemplate); err != nil {
return err
}
if err := assertHasOwnerReference(actualInfrastructureMachineTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
if err := assertHasOwnerReference(actualInfrastructureMachineTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
return err
}

Expand All @@ -280,7 +280,7 @@ func assertMachineDeploymentClass(ctx context.Context, actualClusterClass *clust
if err := env.Get(ctx, actualBootstrapTemplateKey, actualBootstrapTemplate); err != nil {
return err
}
if err := assertHasOwnerReference(actualBootstrapTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
if err := assertHasOwnerReference(actualBootstrapTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
return err
}

Expand Down
9 changes: 6 additions & 3 deletions internal/controllers/clusterclass/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,15 @@ func TestMain(m *testing.M) {
}))
}

func ownerReferenceTo(obj client.Object) *metav1.OwnerReference {
// ownerReferenceTo converts an object to an OwnerReference.
// Note: We pass in gvk explicitly as we can't rely on GVK being set on all objects
// (only on Unstructured).
func ownerReferenceTo(obj client.Object, gvk schema.GroupVersionKind) *metav1.OwnerReference {
return &metav1.OwnerReference{
Kind: obj.GetObjectKind().GroupVersionKind().Kind,
APIVersion: gvk.GroupVersion().String(),
Kind: gvk.Kind,
Name: obj.GetName(),
UID: obj.GetUID(),
APIVersion: obj.GetObjectKind().GroupVersionKind().GroupVersion().String(),
}
}

Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/machine/machine_controller_noderef.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ func (r *Reconciler) reconcileNode(ctx context.Context, s *scope) (ctrl.Result,
// Set the Machine NodeRef.
if machine.Status.NodeRef == nil {
machine.Status.NodeRef = &corev1.ObjectReference{
Kind: node.Kind,
APIVersion: node.APIVersion,
APIVersion: corev1.SchemeGroupVersion.String(),
Kind: "Node",
Name: node.Name,
UID: node.UID,
}
Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/machinedeployment/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ func fakeMachineNodeRef(m *clusterv1.Machine, pid string, g *WithT) {

patchMachine = client.MergeFrom(m.DeepCopy())
m.Status.NodeRef = &corev1.ObjectReference{
APIVersion: node.APIVersion,
Kind: node.Kind,
APIVersion: corev1.SchemeGroupVersion.String(),
Kind: "Node",
Name: node.Name,
}
g.Expect(env.Status().Patch(ctx, m, patchMachine)).To(Succeed())
Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/machineset/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ func fakeMachineNodeRef(m *clusterv1.Machine, pid string, g *WithT) {

patchMachine = client.MergeFrom(m.DeepCopy())
m.Status.NodeRef = &corev1.ObjectReference{
APIVersion: node.APIVersion,
Kind: node.Kind,
APIVersion: corev1.SchemeGroupVersion.String(),
Kind: "Node",
Name: node.Name,
}
g.Expect(env.Status().Patch(ctx, m, patchMachine)).To(Succeed())
Expand Down
25 changes: 14 additions & 11 deletions internal/controllers/topology/cluster/desired_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func computeControlPlaneInfrastructureMachineTemplate(_ context.Context, s *scop
// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
// in case of errors in between creating this template and updating the Cluster object
// with the reference to the ControlPlane object using this template.
ownerRef: ownerReferenceTo(s.Current.Cluster),
ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
})
}

Expand Down Expand Up @@ -623,7 +623,7 @@ func computeMachineDeployment(ctx context.Context, s *scope.Scope, machineDeploy
// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
// in case of errors in between creating this template and creating/updating the MachineDeployment object
// with the reference to this template.
ownerRef: ownerReferenceTo(s.Current.Cluster),
ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -651,7 +651,7 @@ func computeMachineDeployment(ctx context.Context, s *scope.Scope, machineDeploy
// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
// in case of errors in between creating this template and creating/updating the MachineDeployment object
// with the reference to this template.
ownerRef: ownerReferenceTo(s.Current.Cluster),
ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -719,8 +719,8 @@ func computeMachineDeployment(ctx context.Context, s *scope.Scope, machineDeploy

desiredMachineDeploymentObj := &clusterv1.MachineDeployment{
TypeMeta: metav1.TypeMeta{
Kind: clusterv1.GroupVersion.WithKind("MachineDeployment").Kind,
APIVersion: clusterv1.GroupVersion.String(),
Kind: "MachineDeployment",
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand Down Expand Up @@ -977,7 +977,7 @@ func computeMachinePool(_ context.Context, s *scope.Scope, machinePoolTopology c
// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
// in case of errors in between creating this template and creating/updating the MachinePool object
// with the reference to this template.
ownerRef: ownerReferenceTo(s.Current.Cluster),
ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
})
if err != nil {
return nil, errors.Wrapf(err, "failed to compute bootstrap object for topology %q", machinePoolTopology.Name)
Expand Down Expand Up @@ -1005,7 +1005,7 @@ func computeMachinePool(_ context.Context, s *scope.Scope, machinePoolTopology c
// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
// in case of errors in between creating this template and creating/updating the MachinePool object
// with the reference to this template.
ownerRef: ownerReferenceTo(s.Current.Cluster),
ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
})
if err != nil {
return nil, errors.Wrapf(err, "failed to compute infrastructure object for topology %q", machinePoolTopology.Name)
Expand Down Expand Up @@ -1068,8 +1068,8 @@ func computeMachinePool(_ context.Context, s *scope.Scope, machinePoolTopology c

desiredMachinePoolObj := &expv1.MachinePool{
TypeMeta: metav1.TypeMeta{
Kind: expv1.GroupVersion.WithKind("MachinePool").Kind,
APIVersion: expv1.GroupVersion.String(),
Kind: "MachinePool",
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand Down Expand Up @@ -1345,21 +1345,24 @@ func templateToTemplate(in templateToInput) (*unstructured.Unstructured, error)
return template, nil
}

func ownerReferenceTo(obj client.Object) *metav1.OwnerReference {
// ownerReferenceTo converts an object to an OwnerReference.
// Note: We pass in gvk explicitly as we can't rely on GVK being set on all objects
// (only on Unstructured).
func ownerReferenceTo(obj client.Object, gvk schema.GroupVersionKind) *metav1.OwnerReference {
fabriziopandini marked this conversation as resolved.
Show resolved Hide resolved
return &metav1.OwnerReference{
Kind: obj.GetObjectKind().GroupVersionKind().Kind,
APIVersion: gvk.GroupVersion().String(),
Kind: gvk.Kind,
Name: obj.GetName(),
UID: obj.GetUID(),
APIVersion: obj.GetObjectKind().GroupVersionKind().GroupVersion().String(),
}
}

func computeMachineHealthCheck(ctx context.Context, healthCheckTarget client.Object, selector *metav1.LabelSelector, clusterName string, check *clusterv1.MachineHealthCheckClass) *clusterv1.MachineHealthCheck {
// Create a MachineHealthCheck with the spec given in the ClusterClass.
mhc := &clusterv1.MachineHealthCheck{
TypeMeta: metav1.TypeMeta{
Kind: clusterv1.GroupVersion.WithKind("MachineHealthCheck").Kind,
APIVersion: clusterv1.GroupVersion.String(),
Kind: "MachineHealthCheck",
},
ObjectMeta: metav1.ObjectMeta{
Name: healthCheckTarget.GetName(),
Expand Down
6 changes: 3 additions & 3 deletions internal/controllers/topology/cluster/desired_state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func TestComputeInfrastructureCluster(t *testing.T) {
// aggregating current cluster objects into ClusterState (simulating getCurrentState)
scope := scope.New(clusterWithInfrastructureRef)
scope.Current.InfrastructureCluster = infrastructureClusterTemplate.DeepCopy()
scope.Current.InfrastructureCluster.SetOwnerReferences([]metav1.OwnerReference{*ownerReferenceTo(shim)})
scope.Current.InfrastructureCluster.SetOwnerReferences([]metav1.OwnerReference{*ownerReferenceTo(shim, corev1.SchemeGroupVersion.WithKind("Secret"))})
scope.Blueprint = blueprint

obj, err := computeInfrastructureCluster(ctx, scope)
Expand Down Expand Up @@ -623,7 +623,7 @@ func TestComputeControlPlane(t *testing.T) {
}).
Build(),
}
s.Current.ControlPlane.Object.SetOwnerReferences([]metav1.OwnerReference{*ownerReferenceTo(shim)})
s.Current.ControlPlane.Object.SetOwnerReferences([]metav1.OwnerReference{*ownerReferenceTo(shim, corev1.SchemeGroupVersion.WithKind("Secret"))})
s.Blueprint = blueprint

r := &Reconciler{}
Expand Down Expand Up @@ -2877,8 +2877,8 @@ func Test_computeMachineHealthCheck(t *testing.T) {
clusterName := "cluster1"
want := &clusterv1.MachineHealthCheck{
TypeMeta: metav1.TypeMeta{
Kind: clusterv1.GroupVersion.WithKind("MachineHealthCheck").Kind,
APIVersion: clusterv1.GroupVersion.String(),
Kind: "MachineHealthCheck",
},
ObjectMeta: metav1.ObjectMeta{
Name: "md1",
Expand Down
Loading