Skip to content

Commit

Permalink
StatefulSet is sensitive to long names - remove prefix
Browse files Browse the repository at this point in the history
Names in Kubernetes can be up to 253 chars, but labels can only be up to 63 chars.

We are relatively conservative with the two labels we introduce for the Affinity Assistant

    app.kubernetes.io/component: affinity-assistant
    app.kubernetes.io/instance: ws-parallel-pipelinerun-bbx6w

But apparently, StatefulSets adds a label with the full StatefulSet Name + 10 chars (for a hash) as a label

    controller-revision-hash: affinity-assistant-ws-parallel-pipelinerun-bbx6w-dd64c6c8d

This only leave users to use StatefulSet Names up to 53 chars. We use a prefix of 19 chars (affinity-assistant-)
on the Affinity Assistant StatefulSet. This leaves Tekton users with only 34 chars for a combination of
Workspace Name and the PipelineRun Name.

This commit removes the prefix (affinity-assistant-), so that users can use up to 53 chars for
Workspace Name and PipelineRun Name. Also the unnecessary name of the PVC in the volumeClaimTemplate-example is removed.

This limitation of StatefulSet is apparently a known problem kubernetes/kubernetes#64023 but I was not aware of it.

/kind bug
Fixes tektoncd#2766
  • Loading branch information
jlpettersson committed Jun 6, 2020
1 parent 5175c4a commit b923e18
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ spec:
workspaces:
- name: ws
volumeClaimTemplate:
metadata:
name: mypvc
spec:
accessModes:
- ReadWriteOnce
Expand Down
7 changes: 3 additions & 4 deletions pkg/reconciler/pipelinerun/affinity_assistant.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ const (
ReasonCouldntCreateAffinityAssistantStatefulSet = "CouldntCreateAffinityAssistantStatefulSet"

featureFlagDisableAffinityAssistantKey = "disable-affinity-assistant"
affinityAssistantStatefulSetNamePrefix = "affinity-assistant-"
)

// createAffinityAssistants creates an Affinity Assistant StatefulSet for every workspace in the PipelineRun that
Expand All @@ -51,7 +50,7 @@ func (c *Reconciler) createAffinityAssistants(wb []v1alpha1.WorkspaceBinding, pr
for _, w := range wb {
if w.PersistentVolumeClaim != nil || w.VolumeClaimTemplate != nil {
affinityAssistantName := getAffinityAssistantName(w.Name, pr.GetOwnerReference())
affinityAssistantStatefulSetName := affinityAssistantStatefulSetNamePrefix + affinityAssistantName
affinityAssistantStatefulSetName := affinityAssistantName
_, err := c.KubeClientSet.AppsV1().StatefulSets(namespace).Get(affinityAssistantStatefulSetName, metav1.GetOptions{})
claimName := getClaimName(w, pr.GetOwnerReference())
switch {
Expand Down Expand Up @@ -85,7 +84,7 @@ func (c *Reconciler) cleanupAffinityAssistants(pr *v1beta1.PipelineRun) error {
var errs []error
for _, w := range pr.Spec.Workspaces {
if w.PersistentVolumeClaim != nil || w.VolumeClaimTemplate != nil {
affinityAssistantStsName := affinityAssistantStatefulSetNamePrefix + getAffinityAssistantName(w.Name, pr.GetOwnerReference())
affinityAssistantStsName := getAffinityAssistantName(w.Name, pr.GetOwnerReference())
if err := c.KubeClientSet.AppsV1().StatefulSets(pr.Namespace).Delete(affinityAssistantStsName, &metav1.DeleteOptions{}); err != nil {
errs = append(errs, fmt.Errorf("failed to delete StatefulSet %s: %s", affinityAssistantStsName, err))
}
Expand Down Expand Up @@ -169,7 +168,7 @@ func affinityAssistantStatefulSet(name string, pr *v1beta1.PipelineRun, claimNam
APIVersion: "apps/v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: affinityAssistantStatefulSetNamePrefix + name,
Name: name,
Labels: getStatefulSetLabels(pr, name),
OwnerReferences: []metav1.OwnerReference{pr.GetOwnerReference()},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/affinity_assistant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestCreateAndDeleteOfAffinityAssistant(t *testing.T) {
t.Errorf("unexpected error from createAffinityAssistants: %v", err)
}

expectedAffinityAssistantName := affinityAssistantStatefulSetNamePrefix + fmt.Sprintf("%s-%s", workspaceName, pipelineRunName)
expectedAffinityAssistantName := fmt.Sprintf("%s-%s", workspaceName, pipelineRunName)
_, err = c.KubeClientSet.AppsV1().StatefulSets(testPipelineRun.Namespace).Get(expectedAffinityAssistantName, metav1.GetOptions{})
if err != nil {
t.Errorf("unexpected error when retrieving StatefulSet: %v", err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1926,8 +1926,8 @@ func TestReconcileWithAffinityAssistantStatefulSet(t *testing.T) {

expectedAffinityAssistantName1 := fmt.Sprintf("%s-%s", workspaceName, pipelineRunName)
expectedAffinityAssistantName2 := fmt.Sprintf("%s-%s", workspaceName2, pipelineRunName)
expectedStsName1 := affinityAssistantStatefulSetNamePrefix + expectedAffinityAssistantName1
expectedStsName2 := affinityAssistantStatefulSetNamePrefix + expectedAffinityAssistantName2
expectedStsName1 := expectedAffinityAssistantName1
expectedStsName2 := expectedAffinityAssistantName2
expectedAffinityAssistantStsNames := make(map[string]bool)
expectedAffinityAssistantStsNames[expectedStsName1] = true
expectedAffinityAssistantStsNames[expectedStsName2] = true
Expand Down

0 comments on commit b923e18

Please sign in to comment.