diff --git a/examples/v1beta1/pipelineruns/pipeline-run-with-parallel-tasks-using-pvc.yaml b/examples/v1beta1/pipelineruns/pipelinerun-with-parallel-tasks-using-pvc.yaml similarity index 100% rename from examples/v1beta1/pipelineruns/pipeline-run-with-parallel-tasks-using-pvc.yaml rename to examples/v1beta1/pipelineruns/pipelinerun-with-parallel-tasks-using-pvc.yaml diff --git a/examples/v1beta1/pipelineruns/workspace-from-volumeclaimtemplate.yaml b/examples/v1beta1/pipelineruns/workspace-from-volumeclaimtemplate.yaml index 35c325f89b6..62e476d70c5 100644 --- a/examples/v1beta1/pipelineruns/workspace-from-volumeclaimtemplate.yaml +++ b/examples/v1beta1/pipelineruns/workspace-from-volumeclaimtemplate.yaml @@ -41,8 +41,6 @@ spec: workspaces: - name: ws volumeClaimTemplate: - metadata: - name: mypvc spec: accessModes: - ReadWriteOnce diff --git a/pkg/reconciler/pipelinerun/affinity_assistant.go b/pkg/reconciler/pipelinerun/affinity_assistant.go index b3db5fa72e6..09d945242f3 100644 --- a/pkg/reconciler/pipelinerun/affinity_assistant.go +++ b/pkg/reconciler/pipelinerun/affinity_assistant.go @@ -17,6 +17,7 @@ limitations under the License. package pipelinerun import ( + "crypto/sha256" "fmt" "github.com/tektoncd/pipeline/pkg/apis/pipeline" @@ -40,7 +41,6 @@ const ( ReasonCouldntCreateAffinityAssistantStatefulSet = "CouldntCreateAffinityAssistantStatefulSet" featureFlagDisableAffinityAssistantKey = "disable-affinity-assistant" - affinityAssistantStatefulSetNamePrefix = "affinity-assistant-" ) // createAffinityAssistants creates an Affinity Assistant StatefulSet for every workspace in the PipelineRun that @@ -50,9 +50,8 @@ func (c *Reconciler) createAffinityAssistants(wb []v1alpha1.WorkspaceBinding, pr var errs []error for _, w := range wb { if w.PersistentVolumeClaim != nil || w.VolumeClaimTemplate != nil { - affinityAssistantName := getAffinityAssistantName(w.Name, pr.GetOwnerReference()) - affinityAssistantStatefulSetName := affinityAssistantStatefulSetNamePrefix + affinityAssistantName - _, err := c.KubeClientSet.AppsV1().StatefulSets(namespace).Get(affinityAssistantStatefulSetName, metav1.GetOptions{}) + affinityAssistantName := getAffinityAssistantName(w.Name, pr.Name) + _, err := c.KubeClientSet.AppsV1().StatefulSets(namespace).Get(affinityAssistantName, metav1.GetOptions{}) claimName := getClaimName(w, pr.GetOwnerReference()) switch { case apierrors.IsNotFound(err): @@ -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.Name) 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)) } @@ -94,8 +93,10 @@ func (c *Reconciler) cleanupAffinityAssistants(pr *v1beta1.PipelineRun) error { return errorutils.NewAggregate(errs) } -func getAffinityAssistantName(pipelineWorkspaceName string, owner metav1.OwnerReference) string { - return fmt.Sprintf("%s-%s", pipelineWorkspaceName, owner.Name) +func getAffinityAssistantName(pipelineWorkspaceName string, pipelineRunName string) string { + hashBytes := sha256.Sum256([]byte(pipelineWorkspaceName + pipelineRunName)) + hashString := fmt.Sprintf("%x", hashBytes) + return fmt.Sprintf("%s-%s", "affinity-assistant", hashString[:10]) } func getStatefulSetLabels(pr *v1beta1.PipelineRun, affinityAssistantName string) map[string]string { @@ -169,7 +170,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()}, }, diff --git a/pkg/reconciler/pipelinerun/affinity_assistant_test.go b/pkg/reconciler/pipelinerun/affinity_assistant_test.go index 7ed17023c1f..ddb0c8843ba 100644 --- a/pkg/reconciler/pipelinerun/affinity_assistant_test.go +++ b/pkg/reconciler/pipelinerun/affinity_assistant_test.go @@ -17,7 +17,6 @@ limitations under the License. package pipelinerun import ( - "fmt" "testing" "github.com/tektoncd/pipeline/pkg/apis/pipeline" @@ -65,7 +64,7 @@ func TestCreateAndDeleteOfAffinityAssistant(t *testing.T) { t.Errorf("unexpected error from createAffinityAssistants: %v", err) } - expectedAffinityAssistantName := affinityAssistantStatefulSetNamePrefix + fmt.Sprintf("%s-%s", workspaceName, pipelineRunName) + expectedAffinityAssistantName := getAffinityAssistantName(workspaceName, testPipelineRun.Name) _, err = c.KubeClientSet.AppsV1().StatefulSets(testPipelineRun.Namespace).Get(expectedAffinityAssistantName, metav1.GetOptions{}) if err != nil { t.Errorf("unexpected error when retrieving StatefulSet: %v", err) @@ -134,6 +133,22 @@ func TestThatTheAffinityAssistantIsWithoutNodeSelectorAndTolerations(t *testing. } } +// TestThatAffinityAssistantNameIsNoLongerThan53 tests that the Affinity Assistant Name +// is no longer than 53 chars. This is a limitation with StatefulSet. +// See https://github.com/kubernetes/kubernetes/issues/64023 +// This is because the StatefulSet-controller adds a label with the name of the StatefulSet +// plus 10 chars for a hash. Labels in Kubernetes can not be longer than 63 chars. +// Typical output from the example below is affinity-assistant-0384086f62 +func TestThatAffinityAssistantNameIsNoLongerThan53(t *testing.T) { + affinityAssistantName := getAffinityAssistantName( + "pipeline-workspace-name-that-is-quite-long", + "pipelinerun-with-a-long-custom-name") + + if len(affinityAssistantName) > 53 { + t.Errorf("affinity assistant name can not be longer than 53 chars") + } +} + func TestDisableAffinityAssistant(t *testing.T) { for _, tc := range []struct { description string diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 0ff25ec6a53..13e322edb75 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -790,7 +790,7 @@ func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr * } if !c.isAffinityAssistantDisabled() && pipelinePVCWorkspaceName != "" { - tr.Annotations[workspace.AnnotationAffinityAssistantName] = getAffinityAssistantName(pipelinePVCWorkspaceName, pr.GetOwnerReference()) + tr.Annotations[workspace.AnnotationAffinityAssistantName] = getAffinityAssistantName(pipelinePVCWorkspaceName, pr.Name) } resources.WrapSteps(&tr.Spec, rprt.PipelineTask, rprt.ResolvedTaskResources.Inputs, rprt.ResolvedTaskResources.Outputs, storageBasePath) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index b80aefe1d47..e556a7aac01 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -1924,13 +1924,11 @@ func TestReconcileWithAffinityAssistantStatefulSet(t *testing.T) { t.Fatalf("expected one StatefulSet created. %d was created", len(stsNames)) } - expectedAffinityAssistantName1 := fmt.Sprintf("%s-%s", workspaceName, pipelineRunName) - expectedAffinityAssistantName2 := fmt.Sprintf("%s-%s", workspaceName2, pipelineRunName) - expectedStsName1 := affinityAssistantStatefulSetNamePrefix + expectedAffinityAssistantName1 - expectedStsName2 := affinityAssistantStatefulSetNamePrefix + expectedAffinityAssistantName2 + expectedAffinityAssistantName1 := getAffinityAssistantName(workspaceName, pipelineRunName) + expectedAffinityAssistantName2 := getAffinityAssistantName(workspaceName2, pipelineRunName) expectedAffinityAssistantStsNames := make(map[string]bool) - expectedAffinityAssistantStsNames[expectedStsName1] = true - expectedAffinityAssistantStsNames[expectedStsName2] = true + expectedAffinityAssistantStsNames[expectedAffinityAssistantName1] = true + expectedAffinityAssistantStsNames[expectedAffinityAssistantName2] = true for _, stsName := range stsNames { _, found := expectedAffinityAssistantStsNames[stsName] if !found {