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

🐛 clusterctl only deletes the cluster object #1696

Merged
merged 1 commit into from
Oct 31, 2019
Merged
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
58 changes: 36 additions & 22 deletions cmd/clusterctl/clusterdeployer/clusterclient/clusterclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,34 +538,22 @@ func (c *client) CreateMachines(machines []*clusterv1.Machine, namespace string)

// DeleteClusters deletes all Clusters in a namespace. If the namespace is empty then all Clusters in all namespaces are deleted.
func (c *client) DeleteClusters(namespace string) error {
if err := c.clientSet.DeleteAllOf(ctx, &clusterv1.Cluster{}, ctrlclient.InNamespace(namespace)); err != nil {
return errors.Wrapf(err, "error deleting Clusters in namespace %q", namespace)
}
return nil
return c.kubectlDeleteAll(namespace, "clusters")
}

// DeleteMachineDeployments deletes all MachineDeployments in a namespace. If the namespace is empty then all MachineDeployments in all namespaces are deleted.
func (c *client) DeleteMachineDeployments(namespace string) error {
if err := c.clientSet.DeleteAllOf(ctx, &clusterv1.MachineDeployment{}, ctrlclient.InNamespace(namespace)); err != nil {
return errors.Wrapf(err, "error deleting Clusters in namespace %q", namespace)
}
return nil
return c.kubectlDeleteAll(namespace, "machinedeployments")
}

// DeleteMachineSets deletes all MachineSets in a namespace. If the namespace is empty then all MachineSets in all namespaces are deleted.
func (c *client) DeleteMachineSets(namespace string) error {
if err := c.clientSet.DeleteAllOf(ctx, &clusterv1.MachineSet{}, ctrlclient.InNamespace(namespace)); err != nil {
return errors.Wrapf(err, "error deleting Clusters in namespace %q", namespace)
}
return nil
return c.kubectlDeleteAll(namespace, "machinesets")
}

// DeleteMachines deletes all Machines in a namespace. If the namespace is empty then all Machines in all namespaces are deleted.
func (c *client) DeleteMachines(namespace string) error {
if err := c.clientSet.DeleteAllOf(ctx, &clusterv1.Machine{}, ctrlclient.InNamespace(namespace)); err != nil {
return errors.Wrapf(err, "error deleting Clusters in namespace %q", namespace)
}
return nil
return c.kubectlDeleteAll(namespace, "machines")
}

func (c *client) ForceDeleteMachine(namespace, name string) error {
Expand Down Expand Up @@ -777,12 +765,21 @@ func (c *client) kubectlDelete(manifest string) error {
return c.kubectlManifestCmd("delete", manifest)
}

func (c *client) kubectlDeleteAll(namespace, resource string) error {
cmd := exec.Command("kubectl", c.buildKubectlDeleteAllArgs(namespace, resource)...) //nolint:gosec
out, err := cmd.CombinedOutput()
if err != nil {
return errors.Wrapf(err, "couldn't kubectl delete %s --all, output: %s", resource, string(out))
}
return nil
}

func (c *client) kubectlApply(manifest string) error {
return c.kubectlManifestCmd("apply", manifest)
}

func (c *client) kubectlManifestCmd(commandName, manifest string) error {
cmd := exec.Command("kubectl", c.buildKubectlArgs(commandName)...)
cmd := exec.Command("kubectl", c.buildKubectlManifestArgs(commandName)...) //nolint:gosec
cmd.Stdin = strings.NewReader(manifest)
out, err := cmd.CombinedOutput()
if err != nil {
Expand All @@ -791,21 +788,38 @@ func (c *client) kubectlManifestCmd(commandName, manifest string) error {
return nil
}

func (c *client) buildKubectlArgs(commandName string) []string {
func (c *client) buildKubectlManifestArgs(commandName string) []string {
args := []string{commandName}
if c.configOverrides.Context.Namespace != "" {
args = append(args, "--namespace", c.configOverrides.Context.Namespace)
}
args = append(args, c.buildKubectlArgs()...)
return append(args, "-f", "-")
}

func (c *client) buildKubectlDeleteAllArgs(namespace, resource string) []string {
args := []string{"delete", resource}
if namespace == "" {
args = append(args, "--all-namespaces")
} else {
args = append(args, "--namespace", namespace)
}
args = append(args, c.buildKubectlArgs()...)
return append(args, "--all")
}

func (c *client) buildKubectlArgs() []string {
var args []string
if c.kubeconfigFile != "" {
args = append(args, "--kubeconfig", c.kubeconfigFile)
}
if c.configOverrides.Context.Cluster != "" {
args = append(args, "--cluster", c.configOverrides.Context.Cluster)
}
if c.configOverrides.Context.Namespace != "" {
args = append(args, "--namespace", c.configOverrides.Context.Namespace)
}
if c.configOverrides.Context.AuthInfo != "" {
args = append(args, "--user", c.configOverrides.Context.AuthInfo)
}
return append(args, "-f", "-")
return args
}

func (c *client) waitForKubectlApply(manifest string) error {
Expand Down
23 changes: 1 addition & 22 deletions cmd/clusterctl/clusterdeployer/clusterdeployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package clusterdeployer

import (
"fmt"
"strings"

"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -212,29 +211,9 @@ func (d *ClusterDeployer) saveProviderComponentsToCluster(factory provider.Compo
}

func deleteClusterAPIObjectsInAllNamespaces(client clusterclient.Client) error {
var errorList []string
klog.Infof("Deleting MachineDeployments in all namespaces")
if err := client.DeleteMachineDeployments(""); err != nil {
err = errors.Wrap(err, "error deleting MachineDeployments")
errorList = append(errorList, err.Error())
}
klog.Infof("Deleting MachineSets in all namespaces")
if err := client.DeleteMachineSets(""); err != nil {
err = errors.Wrap(err, "error deleting MachineSets")
errorList = append(errorList, err.Error())
}
klog.Infof("Deleting Machines in all namespaces")
if err := client.DeleteMachines(""); err != nil {
err = errors.Wrap(err, "error deleting Machines")
errorList = append(errorList, err.Error())
}
klog.Infof("Deleting Clusters in all namespaces")
if err := client.DeleteClusters(""); err != nil {
err = errors.Wrap(err, "error deleting Clusters")
errorList = append(errorList, err.Error())
}
if len(errorList) > 0 {
return errors.Errorf("error(s) encountered deleting objects from bootstrap cluster: [%v]", strings.Join(errorList, ", "))
return errors.Errorf("error encountered deleting objects from bootstrap cluster: [error deleting Clusters: %v]", err)
}
return nil
}
Expand Down
50 changes: 4 additions & 46 deletions cmd/clusterctl/clusterdeployer/clusterdeployer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1390,45 +1390,21 @@ func TestClusterDelete(t *testing.T) {
expectedErrorMessage: "create cluster error",
expectedExternalClusterCount: 1,
},
{
name: "error deleting machines",
provisionExternalErr: nil,
NewCoreClientsetErr: nil,
bootstrapClient: &testClusterClient{DeleteMachinesErr: errors.New("delete machines error")},
targetClient: &testClusterClient{},
expectedErrorMessage: "error(s) encountered deleting objects from bootstrap cluster: [error deleting Machines: delete machines error]",
},
{
name: "error deleting machine sets",
provisionExternalErr: nil,
NewCoreClientsetErr: nil,
bootstrapClient: &testClusterClient{DeleteMachineSetsErr: errors.New("delete machine sets error")},
targetClient: &testClusterClient{},
expectedErrorMessage: "error(s) encountered deleting objects from bootstrap cluster: [error deleting MachineSets: delete machine sets error]",
},
{
name: "error deleting machine deployments",
provisionExternalErr: nil,
NewCoreClientsetErr: nil,
bootstrapClient: &testClusterClient{DeleteMachineDeploymentsErr: errors.New("delete machine deployments error")},
targetClient: &testClusterClient{},
expectedErrorMessage: "error(s) encountered deleting objects from bootstrap cluster: [error deleting MachineDeployments: delete machine deployments error]",
},
{
name: "error deleting clusters",
provisionExternalErr: nil,
NewCoreClientsetErr: nil,
bootstrapClient: &testClusterClient{DeleteClustersErr: errors.New("delete clusters error")},
targetClient: &testClusterClient{},
expectedErrorMessage: "error(s) encountered deleting objects from bootstrap cluster: [error deleting Clusters: delete clusters error]",
expectedErrorMessage: "error encountered deleting objects from bootstrap cluster: [error deleting Clusters: delete clusters error]",
},
{
name: "error deleting machines and clusters",
provisionExternalErr: nil,
NewCoreClientsetErr: nil,
bootstrapClient: &testClusterClient{DeleteMachinesErr: errors.New("delete machines error"), DeleteClustersErr: errors.New("delete clusters error")},
targetClient: &testClusterClient{},
expectedErrorMessage: "error(s) encountered deleting objects from bootstrap cluster: [error deleting Machines: delete machines error, error deleting Clusters: delete clusters error]",
expectedErrorMessage: "error encountered deleting objects from bootstrap cluster: [error deleting Clusters: delete clusters error]",
},
}

Expand Down Expand Up @@ -1456,21 +1432,12 @@ func TestClusterDelete(t *testing.T) {

if !testCase.expectError {
var (
bootstrapClusters, bootstrapMachines, bootstrapMachineDeployments, bootstrapMachineSets, bootstrapMachineClasses int
targetClusters, targetMachines, targetMachineDeployments, targetMachineSets, targetMachineClasses int
bootstrapClusters, bootstrapMachineClasses int
targetClusters, targetMachines, targetMachineDeployments, targetMachineSets, targetMachineClasses int
)
for _, clusters := range testCase.bootstrapClient.clusters {
bootstrapClusters = bootstrapClusters + len(clusters)
}
for _, machines := range testCase.bootstrapClient.machines {
bootstrapMachines = bootstrapMachines + len(machines)
}
for _, machineDeployments := range testCase.bootstrapClient.machineDeployments {
bootstrapMachineDeployments = bootstrapMachineDeployments + len(machineDeployments)
}
for _, machineSets := range testCase.bootstrapClient.machineSets {
bootstrapMachineSets = bootstrapMachineSets + len(machineSets)
}
for _, clusters := range testCase.targetClient.clusters {
targetClusters = targetClusters + len(clusters)
}
Expand All @@ -1487,15 +1454,6 @@ func TestClusterDelete(t *testing.T) {
if bootstrapClusters != 0 {
t.Fatalf("Unexpected Cluster count in bootstrap cluster. Got: %d, Want: 0", bootstrapClusters)
}
if bootstrapMachines != 0 {
t.Fatalf("Unexpected Machine count in bootstrap cluster. Got: %d, Want: 0", bootstrapMachines)
}
if bootstrapMachineSets != 0 {
t.Fatalf("Unexpected MachineSet count in bootstrap cluster. Got: %d, Want: 0", bootstrapMachineSets)
}
if bootstrapMachineDeployments != 0 {
t.Fatalf("Unexpected MachineDeployment count in bootstrap cluster. Got: %d, Want: 0", bootstrapMachineDeployments)
}
if bootstrapMachineClasses != 0 {
t.Fatalf("Unexpected MachineClass count in bootstrap cluster. Got: %d, Want: 0", bootstrapMachineClasses)
}
Expand Down