From a50ae6d8910fe965a4d1c12843d7e7e7a3f0214d Mon Sep 17 00:00:00 2001 From: Jason DeTiberus Date: Fri, 1 Mar 2019 17:07:02 -0500 Subject: [PATCH] Fix up Machine*/Cluster link and remove hack for MachineDeployment adoption --- .../clusterclient/clusterclient.go | 64 ++++++++++++------- .../clusterdeployer/clusterdeployer_test.go | 6 +- cmd/clusterctl/phases/pivot.go | 48 +------------- 3 files changed, 46 insertions(+), 72 deletions(-) diff --git a/cmd/clusterctl/clusterdeployer/clusterclient/clusterclient.go b/cmd/clusterctl/clusterdeployer/clusterclient/clusterclient.go index 380858451369..f2d134c2a35e 100644 --- a/cmd/clusterctl/clusterdeployer/clusterclient/clusterclient.go +++ b/cmd/clusterctl/clusterdeployer/clusterclient/clusterclient.go @@ -64,14 +64,14 @@ type Client interface { GetClusterObject(string, string) (*clusterv1.Cluster, error) GetMachineClasses(namespace string) ([]*clusterv1.MachineClass, error) GetMachineDeployment(namespace, name string) (*clusterv1.MachineDeployment, error) - GetMachineDeploymentsForCluster(namespace, name string) ([]*clusterv1.MachineDeployment, error) + GetMachineDeploymentsForCluster(*clusterv1.Cluster) ([]*clusterv1.MachineDeployment, error) GetMachineDeployments(string) ([]*clusterv1.MachineDeployment, error) GetMachineSet(namespace, name string) (*clusterv1.MachineSet, error) GetMachineSets(namespace string) ([]*clusterv1.MachineSet, error) - GetMachineSetsForCluster(namespace, name string) ([]*clusterv1.MachineSet, error) + GetMachineSetsForCluster(*clusterv1.Cluster) ([]*clusterv1.MachineSet, error) GetMachineSetsForMachineDeployment(*clusterv1.MachineDeployment) ([]*clusterv1.MachineSet, error) GetMachines(namespace string) ([]*clusterv1.Machine, error) - GetMachinesForCluster(namespace, name string) ([]*clusterv1.Machine, error) + GetMachinesForCluster(*clusterv1.Cluster) ([]*clusterv1.Machine, error) GetMachinesForMachineSet(*clusterv1.MachineSet) ([]*clusterv1.Machine, error) CreateClusterObject(*clusterv1.Cluster) error CreateMachineClass(*clusterv1.MachineClass) error @@ -282,16 +282,21 @@ func (c *client) GetMachineDeployment(namespace, name string) (*clusterv1.Machin return machineDeployment, nil } -func (c *client) GetMachineDeploymentsForCluster(namespace, name string) ([]*clusterv1.MachineDeployment, error) { - machineDeploymentList, err := c.clientSet.ClusterV1alpha1().MachineDeployments(namespace).List(metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=%s", machineClusterLabelName, name), +func (c *client) GetMachineDeploymentsForCluster(cluster *clusterv1.Cluster) ([]*clusterv1.MachineDeployment, error) { + machineDeploymentList, err := c.clientSet.ClusterV1alpha1().MachineDeployments(cluster.Namespace).List(metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", machineClusterLabelName, cluster.Name), }) if err != nil { - return nil, errors.Wrapf(err, "error listing MachineDeployments in namespace %q", namespace) + return nil, errors.Wrapf(err, "error listing MachineDeployments for Cluster %s/%s", cluster.Namespace, cluster.Name) } var machineDeployments []*clusterv1.MachineDeployment - for i := 0; i < len(machineDeploymentList.Items); i++ { - machineDeployments = append(machineDeployments, &machineDeploymentList.Items[i]) + for _, md := range machineDeploymentList.Items { + for _, or := range md.GetOwnerReferences() { + if or.Kind == cluster.Kind && or.Name == cluster.Name { + machineDeployments = append(machineDeployments, &md) + continue + } + } } return machineDeployments, nil } @@ -326,7 +331,7 @@ func (c *client) GetMachineSet(namespace, name string) (*clusterv1.MachineSet, e func (c *client) GetMachineSets(namespace string) ([]*clusterv1.MachineSet, error) { machineSetList, err := c.clientSet.ClusterV1alpha1().MachineSets(namespace).List(metav1.ListOptions{}) if err != nil { - return nil, errors.Wrapf(err, "error listing machine set objects in namespace %q", namespace) + return nil, errors.Wrapf(err, "error listing MachineSets in namespace %q", namespace) } var machineSets []*clusterv1.MachineSet for i := 0; i < len(machineSetList.Items); i++ { @@ -335,19 +340,23 @@ func (c *client) GetMachineSets(namespace string) ([]*clusterv1.MachineSet, erro return machineSets, nil } -func (c *client) GetMachineSetsForCluster(namespace, name string) ([]*clusterv1.MachineSet, error) { - machineSetList, err := c.clientSet.ClusterV1alpha1().MachineSets(namespace).List(metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=%s", machineClusterLabelName, name), +func (c *client) GetMachineSetsForCluster(cluster *clusterv1.Cluster) ([]*clusterv1.MachineSet, error) { + machineSetList, err := c.clientSet.ClusterV1alpha1().MachineSets(cluster.Namespace).List(metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", machineClusterLabelName, cluster.Name), }) if err != nil { - return nil, errors.Wrapf(err, "error listing machine set objects in namespace %q", namespace) + return nil, errors.Wrapf(err, "error listing MachineSets for Cluster %s/%s", cluster.Namespace, cluster.Name) } var machineSets []*clusterv1.MachineSet - for i := 0; i < len(machineSetList.Items); i++ { - machineSets = append(machineSets, &machineSetList.Items[i]) + for _, ms := range machineSetList.Items { + for _, or := range ms.GetOwnerReferences() { + if or.Kind == cluster.Kind && or.Name == cluster.Name { + machineSets = append(machineSets, &ms) + continue + } + } } return machineSets, nil - } func (c *client) GetMachineSetsForMachineDeployment(md *clusterv1.MachineDeployment) ([]*clusterv1.MachineSet, error) { @@ -368,7 +377,7 @@ func (c *client) GetMachines(namespace string) ([]*clusterv1.Machine, error) { machines := []*clusterv1.Machine{} machineslist, err := c.clientSet.ClusterV1alpha1().Machines(namespace).List(metav1.ListOptions{}) if err != nil { - return nil, errors.Wrapf(err, "error listing machine objects in namespace %q", namespace) + return nil, errors.Wrapf(err, "error listing Machines in namespace %q", namespace) } for i := 0; i < len(machineslist.Items); i++ { @@ -377,15 +386,22 @@ func (c *client) GetMachines(namespace string) ([]*clusterv1.Machine, error) { return machines, nil } -func (c *client) GetMachinesForCluster(namespace, name string) ([]*clusterv1.Machine, error) { - machines := []*clusterv1.Machine{} - machineslist, err := c.clientSet.ClusterV1alpha1().Machines(namespace).List(metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=%s", machineClusterLabelName, name), +func (c *client) GetMachinesForCluster(cluster *clusterv1.Cluster) ([]*clusterv1.Machine, error) { + machineslist, err := c.clientSet.ClusterV1alpha1().Machines(cluster.Namespace).List(metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", machineClusterLabelName, cluster.Name), }) if err != nil { - return nil, errors.Wrapf(err, "error listing machine objects in namespace %q", namespace) + return nil, errors.Wrapf(err, "error listing Machines for Cluster %s/%s", cluster.Namespace, cluster.Name) + } + var machines []*clusterv1.Machine + for _, m := range machineslist.Items { + for _, or := range m.GetOwnerReferences() { + if or.Kind == cluster.Kind && or.Name == cluster.Name { + machines = append(machines, &m) + continue + } + } } - for i := 0; i < len(machineslist.Items); i++ { machines = append(machines, &machineslist.Items[i]) } diff --git a/cmd/clusterctl/clusterdeployer/clusterdeployer_test.go b/cmd/clusterctl/clusterdeployer/clusterdeployer_test.go index f3c251eb62e0..cb238b76a7f4 100644 --- a/cmd/clusterctl/clusterdeployer/clusterdeployer_test.go +++ b/cmd/clusterctl/clusterdeployer/clusterdeployer_test.go @@ -325,7 +325,7 @@ func (c *testClusterClient) GetMachinesForMachineSet(*clusterv1.MachineSet) ([]* } // TODO: implement GetMachineSetsForCluster for testClusterClient and add tests -func (c *testClusterClient) GetMachineSetsForCluster(namespace, name string) ([]*clusterv1.MachineSet, error) { +func (c *testClusterClient) GetMachineSetsForCluster(*clusterv1.Cluster) ([]*clusterv1.MachineSet, error) { return nil, errors.Errorf("Not yet implemented.") } @@ -335,7 +335,7 @@ func (c *testClusterClient) GetMachineDeployment(namespace, name string) (*clust } // TODO: implement GetMachineDeploymentsForCluster for testClusterClient and add tests -func (c *testClusterClient) GetMachineDeploymentsForCluster(namespace, name string) ([]*clusterv1.MachineDeployment, error) { +func (c *testClusterClient) GetMachineDeploymentsForCluster(*clusterv1.Cluster) ([]*clusterv1.MachineDeployment, error) { return nil, errors.Errorf("Not yet implemented.") } @@ -345,7 +345,7 @@ func (c *testClusterClient) GetMachineSet(namespace, name string) (*clusterv1.Ma } // TODO: implement GetMachinesForCluster for testClusterClient and add tests -func (c *testClusterClient) GetMachinesForCluster(namespace, name string) ([]*clusterv1.Machine, error) { +func (c *testClusterClient) GetMachinesForCluster(*clusterv1.Cluster) ([]*clusterv1.Machine, error) { return nil, errors.Errorf("Not yet implemented.") } diff --git a/cmd/clusterctl/phases/pivot.go b/cmd/clusterctl/phases/pivot.go index e54e6de48980..52bd1fa3ad71 100644 --- a/cmd/clusterctl/phases/pivot.go +++ b/cmd/clusterctl/phases/pivot.go @@ -22,7 +22,6 @@ import ( "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/klog" "sigs.k8s.io/cluster-api/cmd/clusterctl/clusterdeployer/clusterclient" @@ -48,8 +47,6 @@ func Pivot(source, target clusterclient.Client, providerComponents string) error func pivot(from, to clusterclient.Client, providerComponents string) error { // TODO: Attempt to handle rollback in case of pivot failure - // TODO: Add support for migrating MachineClass objects - // TODO: filter for objects that should not be pivoted klog.V(4).Info("Ensuring cluster v1alpha1 resources are available on the source cluster") if err := from.WaitForClusterV1alpha1Ready(); err != nil { @@ -215,10 +212,8 @@ func moveCluster(from, to clusterclient.Client, cluster *clusterv1.Cluster) erro return errors.Wrapf(err, "error copying Cluster %s/%s to target cluster", cluster.Namespace, cluster.Name) } - // TODO: Once there is a more formal link between Machine* objects and Clusters this will need to be - // updated, currently they are linked through existing in the same Namespace. klog.V(4).Infof("Retrieving list of MachineDeployments to move for Cluster %s/%s", cluster.Namespace, cluster.Name) - machineDeployments, err := from.GetMachineDeploymentsForCluster(cluster.Namespace, cluster.Name) + machineDeployments, err := from.GetMachineDeploymentsForCluster(cluster) if err != nil { return err } @@ -227,7 +222,7 @@ func moveCluster(from, to clusterclient.Client, cluster *clusterv1.Cluster) erro } klog.V(4).Infof("Retrieving list of MachineSets not associated with a MachineDeployment to move for Cluster %s/%s", cluster.Namespace, cluster.Name) - machineSets, err := from.GetMachineSetsForCluster(cluster.Namespace, cluster.Name) + machineSets, err := from.GetMachineSetsForCluster(cluster) if err != nil { return err } @@ -236,7 +231,7 @@ func moveCluster(from, to clusterclient.Client, cluster *clusterv1.Cluster) erro } klog.V(4).Infof("Retrieving list of Machines not associated with a MachineSet to move for Cluster %s/%s", cluster.Namespace, cluster.Name) - machines, err := from.GetMachinesForCluster(cluster.Namespace, cluster.Name) + machines, err := from.GetMachinesForCluster(cluster) if err != nil { return err } @@ -289,12 +284,6 @@ func moveMachineDeployment(from, to clusterclient.Client, md *clusterv1.MachineD return errors.Wrapf(err, "error copying MachineDeployment %s/%s to target cluster", md.Namespace, md.Name) } - // Since MachineDeployments do not currently adopt pre-existing MachineSets, we need - // to patch up the owner refs for the MachineSets we created: https://github.com/kubernetes-sigs/cluster-api/issues/746 - if err := patchMachineDeploymentOwnerRefs(to, md, machineSets); err != nil { - return err - } - if err := from.ForceDeleteMachineDeployment(md.Namespace, md.Name); err != nil { return errors.Wrapf(err, "error force deleting MachineDeployment %s/%s from source cluster", md.Namespace, md.Name) } @@ -302,37 +291,6 @@ func moveMachineDeployment(from, to clusterclient.Client, md *clusterv1.MachineD return nil } -func patchMachineDeploymentOwnerRefs(client clusterclient.Client, md *clusterv1.MachineDeployment, machineSets []*clusterv1.MachineSet) error { - newMD, err := client.GetMachineDeployment(md.Namespace, md.Name) - if err != nil { - return errors.Wrapf(err, "error retrieving newly created MachineDeployment %s/%s from target cluster", md.Namespace, md.Name) - } - for _, ms := range machineSets { - newMS, err := client.GetMachineSet(ms.Namespace, ms.Name) - if err != nil { - return errors.Wrapf(err, "error retrieving newly created MachineSet %s/%s from target cluster", ms.Namespace, ms.Name) - } - if err := updateMachineSetOwnerRefs(client, newMS, newMD); err != nil { - return err - } - } - return nil -} - -func updateMachineSetOwnerRefs(client clusterclient.Client, ms *clusterv1.MachineSet, md *clusterv1.MachineDeployment) error { - ownerRefs := ms.GetOwnerReferences() - if ownerRefs == nil { - ownerRefs = []metav1.OwnerReference{} - } - newRef := *metav1.NewControllerRef(md, clusterv1.SchemeGroupVersion.WithKind("MachineDeployment")) - ownerRefs = append(ownerRefs, newRef) - ms.ObjectMeta.SetOwnerReferences(ownerRefs) - if err := client.UpdateMachineSet(ms); err != nil { - return errors.Wrapf(err, "error updating owner refs for MachineSet: %s/%s", ms.Namespace, ms.Name) - } - return nil -} - func moveMachineSets(from, to clusterclient.Client, machineSets []*clusterv1.MachineSet) error { machineSetNames := make([]string, 0, len(machineSets)) for _, ms := range machineSets {