Skip to content
This repository has been archived by the owner on Mar 28, 2020. It is now read-only.

Commit

Permalink
pkg/*: Fix long cluster names causing pods to hang
Browse files Browse the repository at this point in the history
This patch restricts the name of the cluster to less than 52 characters. This simpler approach is preferred so it can be documented. Operator log displays appropriate error for users.

Fixes #1994
  • Loading branch information
alaypatel07 authored and hexfusion committed Jan 8, 2019
1 parent 2cd243b commit cb35c2d
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 4 deletions.
3 changes: 3 additions & 0 deletions pkg/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ type Cluster struct {

func New(config Config, cl *api.EtcdCluster) *Cluster {
lg := logrus.WithField("pkg", "cluster").WithField("cluster-name", cl.Name).WithField("cluster-namespace", cl.Namespace)
if len(cl.Name) > k8sutil.MaxNameLength || len(cl.ClusterName) > k8sutil.MaxNameLength {
return nil
}

c := &Cluster{
logger: lg,
Expand Down
16 changes: 16 additions & 0 deletions pkg/cluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,19 @@ func TestUpdateEventUpdateLocalClusterObj(t *testing.T) {
t.Errorf("expect version=%s, get=%s", newVersion, c.cluster.ResourceVersion)
}
}

func TestNewLongClusterName(t *testing.T) {
clus := &api.EtcdCluster{
TypeMeta: metav1.TypeMeta{
APIVersion: api.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "example-etcd-cluster123456789123456789123456789123456789123456",
Namespace: metav1.NamespaceDefault,
},
}
clus.SetClusterName("example-etcd-cluster123456789123456789123456789123456789123456")
if c := New(Config{}, clus); c != nil {
t.Errorf("expect c to be nil")
}
}
4 changes: 3 additions & 1 deletion pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ func (c *Controller) handleClusterEvent(event *Event) (bool, error) {
}

nc := cluster.New(c.makeClusterConfig(), clus)

if nc == nil {
return false, fmt.Errorf("cluster name cannot be more than %v characters long, please delete the CR\n", k8sutil.MaxNameLength)
}
c.clusters[getNamespacedName(clus)] = nc

clustersCreated.Inc()
Expand Down
21 changes: 21 additions & 0 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,24 @@ func TestHandleClusterEventNamespacedIgnored(t *testing.T) {
t.Errorf("cluster should be ignored")
}
}

func TestHandleClusterEventWithLongClusterName(t *testing.T) {
c := New(Config{})

clus := &api.EtcdCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "example-etcd-cluster123456789123456789123456789123456789123456",
},
}
e := &Event{
Type: watch.Added,
Object: clus,
}
if ignored, err := c.handleClusterEvent(e); !ignored {
if err == nil {
t.Errorf("err should not be nil")
}
} else {
t.Errorf("cluster should not be ignored")
}
}
6 changes: 3 additions & 3 deletions pkg/util/k8sutil/k8sutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const (

randomSuffixLength = 10
// k8s object name has a maximum length
maxNameLength = 63 - randomSuffixLength - 1
MaxNameLength = 63 - randomSuffixLength - 1

defaultBusyboxImage = "busybox:1.28.0-glibc"

Expand Down Expand Up @@ -517,8 +517,8 @@ func mergeLabels(l1, l2 map[string]string) {

func UniqueMemberName(clusterName string) string {
suffix := utilrand.String(randomSuffixLength)
if len(clusterName) > maxNameLength {
clusterName = clusterName[:maxNameLength]
if len(clusterName) > MaxNameLength {
clusterName = clusterName[:MaxNameLength]
}
return clusterName + "-" + suffix
}

0 comments on commit cb35c2d

Please sign in to comment.