From 3aa185e8f4aa9b40d39712842ae40b7e0e2fad16 Mon Sep 17 00:00:00 2001 From: Ashish Amarnath Date: Sat, 29 Jun 2019 17:50:52 -0700 Subject: [PATCH 1/2] Cluster reconciliation should reconcile exited ELB * get only running ELB containers * remove exited ELB containers with name conflict Signed-off-by: Ashish Amarnath --- actuators/actuators.go | 7 +++++++ actuators/cluster.go | 1 + kind/actions/kind.go | 24 ++++++++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/actuators/actuators.go b/actuators/actuators.go index 025f1d8..7c26003 100644 --- a/actuators/actuators.go +++ b/actuators/actuators.go @@ -29,6 +29,10 @@ import ( "sigs.k8s.io/kind/pkg/cluster/nodes" ) +const ( + containerRunningStatus = "running" +) + func getRole(machine *clusterv1.Machine) string { // Figure out what kind of node we're making labels := machine.GetLabels() @@ -40,9 +44,11 @@ func getRole(machine *clusterv1.Machine) string { } func getExternalLoadBalancerNode(clusterName string) (*nodes.Node, error) { + fmt.Printf("Getting external load balancer node for cluster %q\n", clusterName) elb, err := nodes.List( fmt.Sprintf("label=%s=%s", constants.NodeRoleKey, constants.ExternalLoadBalancerNodeRoleValue), fmt.Sprintf("label=%s=%s", constants.ClusterLabelKey, clusterName), + fmt.Sprintf("status=%s", containerRunningStatus), ) if err != nil { return nil, err @@ -53,6 +59,7 @@ func getExternalLoadBalancerNode(clusterName string) (*nodes.Node, error) { if len(elb) > 1 { return nil, errors.New("too many external load balancers") } + fmt.Printf("External loadbalancer node for cluster %q is %v\n", clusterName, elb[0]) return &elb[0], nil } diff --git a/actuators/cluster.go b/actuators/cluster.go index 9f92eb3..1d338cf 100644 --- a/actuators/cluster.go +++ b/actuators/cluster.go @@ -29,6 +29,7 @@ type Cluster struct { // Reconcile setups an external load balancer for the cluster if needed func (c *Cluster) Reconcile(cluster *clusterv1.Cluster) error { + fmt.Printf("Reconciling cluster %s/%s\n", cluster.Namespace, cluster.Name) elb, err := getExternalLoadBalancerNode(cluster.Name) if err != nil { c.Log.Error(err, "Error getting external load balancer node") diff --git a/kind/actions/kind.go b/kind/actions/kind.go index 3eb689a..eb96d10 100644 --- a/kind/actions/kind.go +++ b/kind/actions/kind.go @@ -71,10 +71,34 @@ func AddControlPlane(clusterName, machineName, version string) (*nodes.Node, err return controlPlane, nil } +func removeExitedELBWithNameConflict(name string) error { + exitedLB, err := nodes.List( + fmt.Sprintf("label=%s=%s", constants.NodeRoleKey, constants.ExternalLoadBalancerNodeRoleValue), + fmt.Sprintf("name=%s", name), + fmt.Sprintf("status=exited"), + ) + + if err != nil { + return errors.Wrapf(err, "failed to list exited external load balancer nodes with name %q", name) + } + + if len(exitedLB) > 0 { + // only one container with given name should exist, if any. + fmt.Printf("Removing exited ELB %q\n", exitedLB[0].Name()) + return nodes.Delete(exitedLB[0]) + } + return nil +} + // SetUpLoadBalancer creates a load balancer but does not configure it. func SetUpLoadBalancer(clusterName string) (*nodes.Node, error) { clusterLabel := fmt.Sprintf("%s=%s", constants.ClusterLabelKey, clusterName) name := fmt.Sprintf("%s-%s", clusterName, constants.ExternalLoadBalancerNodeRoleValue) + err := removeExitedELBWithNameConflict(name) + if err != nil { + return nil, errors.Wrapf(err, "failed to delete exited load balancer node %q", name) + } + return nodes.CreateExternalLoadBalancerNode( name, loadbalancer.Image, From 603fffb57d1ae124e5b8ad1adff27b0926f98aa4 Mon Sep 17 00:00:00 2001 From: Ashish Amarnath Date: Tue, 2 Jul 2019 23:32:04 -0700 Subject: [PATCH 2/2] use logr instead of fmt printfs --- actuators/actuators.go | 7 ++++--- actuators/cluster.go | 4 ++-- actuators/machine.go | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/actuators/actuators.go b/actuators/actuators.go index 7c26003..c1bd5dc 100644 --- a/actuators/actuators.go +++ b/actuators/actuators.go @@ -20,6 +20,7 @@ import ( "fmt" "io/ioutil" + "github.com/go-logr/logr" "github.com/pkg/errors" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -43,8 +44,8 @@ func getRole(machine *clusterv1.Machine) string { return setValue } -func getExternalLoadBalancerNode(clusterName string) (*nodes.Node, error) { - fmt.Printf("Getting external load balancer node for cluster %q\n", clusterName) +func getExternalLoadBalancerNode(clusterName string, log logr.Logger) (*nodes.Node, error) { + log.Info("Getting external load balancer node for cluster", "cluster-name", clusterName) elb, err := nodes.List( fmt.Sprintf("label=%s=%s", constants.NodeRoleKey, constants.ExternalLoadBalancerNodeRoleValue), fmt.Sprintf("label=%s=%s", constants.ClusterLabelKey, clusterName), @@ -59,7 +60,7 @@ func getExternalLoadBalancerNode(clusterName string) (*nodes.Node, error) { if len(elb) > 1 { return nil, errors.New("too many external load balancers") } - fmt.Printf("External loadbalancer node for cluster %q is %v\n", clusterName, elb[0]) + log.Info("External loadbalancer node for cluster", "cluster-name", clusterName, "elb", elb[0]) return &elb[0], nil } diff --git a/actuators/cluster.go b/actuators/cluster.go index 1d338cf..ba85293 100644 --- a/actuators/cluster.go +++ b/actuators/cluster.go @@ -29,8 +29,8 @@ type Cluster struct { // Reconcile setups an external load balancer for the cluster if needed func (c *Cluster) Reconcile(cluster *clusterv1.Cluster) error { - fmt.Printf("Reconciling cluster %s/%s\n", cluster.Namespace, cluster.Name) - elb, err := getExternalLoadBalancerNode(cluster.Name) + c.Log.Info("Reconciling cluster", "cluster-namespace", cluster.Namespace, "cluster-name", cluster.Name) + elb, err := getExternalLoadBalancerNode(cluster.Name, c.Log) if err != nil { c.Log.Error(err, "Error getting external load balancer node") return err diff --git a/actuators/machine.go b/actuators/machine.go index e94b639..aa2d53f 100644 --- a/actuators/machine.go +++ b/actuators/machine.go @@ -92,7 +92,7 @@ func (m *Machine) Create(ctx context.Context, c *clusterv1.Cluster, machine *clu } m.Log.Info("Creating a brand new cluster") - elb, err := getExternalLoadBalancerNode(c.Name) + elb, err := getExternalLoadBalancerNode(c.Name, m.Log) if err != nil { m.Log.Error(err, "Error getting external load balancer node") return err