From 0b7ead8e16ae5353667016df3a03e801a4f3855c Mon Sep 17 00:00:00 2001 From: pritidesai Date: Sat, 6 May 2023 22:36:32 -0700 Subject: [PATCH] update affinity assistant creation implementation Before this commit, the affinity assistant was created in the beginning of the pipleineRun. And the same affinity assistant was relied on for the entire lifecycle of a PR. Now, there could be a case when the node on which affinity assistant pod is created goes down. In this case, the rest of the pipelineRun is stuck and cannot run to the completition since the affinity assistant (StatefulSet) tries to schedule rest of the pods (taskRuns) on the same node but that node is cordoned or not scheduling anything new. This commit always makes an attempt to create Affinity Assistant (StatefulSet) in case it does not exist. If it exist, the controller checks if the node on which Affinity Assistant pod is created is healthy to schedule subsequent pods. If not, the controller deletes Affinity Assistant pod so that StatefulSet can upscale the replicas (set to 1) on other node in the cluster. Signed-off-by: Priti Desai --- config/200-clusterrole.yaml | 4 + docs/additional-configs.md | 6 + docs/developers/README.md | 1 + docs/developers/affinity-assistant.md | 125 +++++++++++ docs/workspaces.md | 6 + .../pipelinerun/affinity_assistant.go | 49 ++++- .../pipelinerun/affinity_assistant_test.go | 205 +++++++++++++++--- pkg/reconciler/pipelinerun/pipelinerun.go | 20 +- 8 files changed, 367 insertions(+), 49 deletions(-) create mode 100644 docs/developers/affinity-assistant.md diff --git a/config/200-clusterrole.yaml b/config/200-clusterrole.yaml index e95592832ad..4acffe6aa6a 100644 --- a/config/200-clusterrole.yaml +++ b/config/200-clusterrole.yaml @@ -25,6 +25,10 @@ rules: # Controller needs to watch Pods created by TaskRuns to see them progress. resources: ["pods"] verbs: ["list", "watch"] + - apiGroups: [""] + # Controller needs to get the list of cordoned nodes over the course of a single run + resources: ["nodes"] + verbs: ["list"] # Controller needs cluster access to all of the CRDs that it is responsible for # managing. - apiGroups: ["tekton.dev"] diff --git a/docs/additional-configs.md b/docs/additional-configs.md index f0f3021d3e7..ea64a6a27a9 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -30,6 +30,7 @@ installation. - [Verify signatures using `cosign`](#verify-signatures-using-cosign) - [Verify the transparency logs using `rekor-cli`](#verify-the-transparency-logs-using-rekor-cli) - [Verify Tekton Resources](#verify-tekton-resources) + - [Pipelinerun with Affinity Assistant](#pipelineruns-with-affinity-assistant) - [Next steps](#next-steps) @@ -554,6 +555,11 @@ Trusted Resources is a feature to verify Tekton Tasks and Pipelines. The current version of feature supports `v1beta1` `Task` and `Pipeline`. For more details please take a look at [Trusted Resources](./trusted-resources.md). +## Pipelineruns with Affinity Assistant + +The cluster operators can review the [guidelines](developers/affinity-assistant.md) to `cordon` a node in the cluster +with the tekton controller and the affinity assistant is enabled. + ## Next steps To get started with Tekton check the [Introductory tutorials][quickstarts], diff --git a/docs/developers/README.md b/docs/developers/README.md index 2e71bfb1e5e..7e2a395b495 100644 --- a/docs/developers/README.md +++ b/docs/developers/README.md @@ -21,3 +21,4 @@ channel for training and tutorials on Tekton! - How specific features are implemented: - [PipelineResources (deprecated)](./pipelineresources.md) - [Results](./results-lifecycle.md) + - [Affinity Assistant](./affinity-assistant.md) diff --git a/docs/developers/affinity-assistant.md b/docs/developers/affinity-assistant.md new file mode 100644 index 00000000000..5019858be32 --- /dev/null +++ b/docs/developers/affinity-assistant.md @@ -0,0 +1,125 @@ +# Affinity Assistant + + +[Specifying `Workspaces` in a `Pipeline`](../workspaces.md#specifying-workspace-order-in-a-pipeline-and-affinity-assistants) explains +how an affinity assistant is created when a `persistentVolumeClaim` is used as a volume source for a `workspace` in a `pipelineRun`. +Please refer to the same section for more details on the affinity assistant. + +This section gives an overview of how the affinity assistant is resilient to a cluster maintenance without losing +the running `pipelineRun`. (Please refer to the issue https://github.com/tektoncd/pipeline/issues/6586 for more details.) + +When a list of `tasks` share a single workspace, the affinity assistant pod gets created on a `node` along with all +`taskRun` pods. It is very common for a `pipeline` author to design a long-running tasks with a single workspace. +With these long-running tasks, a `node` on which these pods are scheduled can be cordoned while the `pipelineRun` is +still running. The tekton controller migrates the affinity assistant pod to any available `node` in a cluster along with +the rest of the `taskRun` pods sharing the same workspace. + +Let's understand this with a sample `pipelineRun`: + +```yaml +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + generateName: pipeline-run- +spec: + workspaces: + - name: source + volumeClaimTemplate: + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 10Mi + pipelineSpec: + workspaces: + - name: source + tasks: + - name: first-task + taskSpec: + workspaces: + - name: source + steps: + - image: alpine + script: | + echo $(workspaces.source.path) + sleep 60 + workspaces: + - name: source + - name: last-task + taskSpec: + workspaces: + - name: source + steps: + - image: alpine + script: | + echo $(workspaces.source.path) + sleep 60 + runAfter: ["first-task"] + workspaces: + - name: source +``` + +This `pipelineRun` has two long-running tasks, `first-task` and `last-task`. Both of these tasks are sharing a single +volume with the access mode set to `ReadWriteOnce` which means the volume can be mounted to a single `node` at any +given point of time. + +Create a `pipelineRun` and determine on which `node` the affinity assistant pod is scheduled: + +```shell +kubectl get pods -l app.kubernetes.io/component=affinity-assistant -o wide -w +NAME READY STATUS RESTARTS AGE IP NODE NOMINATED NODE READINESS GATES +affinity-assistant-c7b485007a-0 0/1 Pending 0 0s +affinity-assistant-c7b485007a-0 0/1 Pending 0 0s kind-multinode-worker1 +affinity-assistant-c7b485007a-0 0/1 ContainerCreating 0 0s kind-multinode-worker1 +affinity-assistant-c7b485007a-0 0/1 ContainerCreating 0 1s kind-multinode-worker1 +affinity-assistant-c7b485007a-0 1/1 Running 0 5s 10.244.1.144 kind-multinode-worker1 +``` + +Now, `cordon` that node to mark it unschedulable for any new pods: + +```shell +kubectl cordon kind-multinode-worker1 +node/kind-multinode-worker1 cordoned +``` + +The node is cordoned: + +```shell +kubectl get node +NAME STATUS ROLES AGE VERSION +kind-multinode-control-plane Ready control-plane 13d v1.26.3 +kind-multinode-worker1 Ready,SchedulingDisabled 13d v1.26.3 +kind-multinode-worker2 Ready 13d v1.26.3 +``` + +Now, watch the affinity assistant pod getting transferred onto other available node `kind-multinode-worker2`: + +```shell +kubectl get pods -l app.kubernetes.io/component=affinity-assistant -o wide -w +NAME READY STATUS RESTARTS AGE IP NODE NOMINATED NODE READINESS GATES +affinity-assistant-c7b485007a-0 1/1 Running 0 49s 10.244.1.144 kind-multinode-worker1 +affinity-assistant-c7b485007a-0 1/1 Terminating 0 70s 10.244.1.144 kind-multinode-worker1 +affinity-assistant-c7b485007a-0 1/1 Terminating 0 70s 10.244.1.144 kind-multinode-worker1 +affinity-assistant-c7b485007a-0 0/1 Terminating 0 70s 10.244.1.144 kind-multinode-worker1 +affinity-assistant-c7b485007a-0 0/1 Terminating 0 70s 10.244.1.144 kind-multinode-worker1 +affinity-assistant-c7b485007a-0 0/1 Terminating 0 70s 10.244.1.144 kind-multinode-worker1 +affinity-assistant-c7b485007a-0 0/1 Pending 0 0s +affinity-assistant-c7b485007a-0 0/1 Pending 0 1s kind-multinode-worker2 +affinity-assistant-c7b485007a-0 0/1 ContainerCreating 0 1s kind-multinode-worker2 +affinity-assistant-c7b485007a-0 0/1 ContainerCreating 0 2s kind-multinode-worker2 +affinity-assistant-c7b485007a-0 1/1 Running 0 4s 10.244.2.144 kind-multinode-worker2 +``` + +And, the `pipelineRun` finishes to completion: + +```shell +kubectl get pr +NAME SUCCEEDED REASON STARTTIME COMPLETIONTIME +pipeline-run-r2c7k True Succeeded 4m22s 2m1s + +kubectl get tr +NAME SUCCEEDED REASON STARTTIME COMPLETIONTIME +pipeline-run-r2c7k-first-task True Succeeded 5m16s 4m7s +pipeline-run-r2c7k-last-task True Succeeded 4m6s 2m56s +``` diff --git a/docs/workspaces.md b/docs/workspaces.md index 150d2040a8d..c9803570041 100644 --- a/docs/workspaces.md +++ b/docs/workspaces.md @@ -381,6 +381,12 @@ significantly. We do not recommend using them in clusters larger than several hu node in the cluster must have an appropriate label matching `topologyKey`. If some or all nodes are missing the specified `topologyKey` label, it can lead to unintended behavior. +**Note:** Any time during the execution of a `pipelineRun`, if the node with a placeholder Affinity Assistant pod and +the `taskRun` pods sharing a `workspace` is `cordoned` or disabled for scheduling anything new (`tainted`), the +`pipelineRun` controller deletes the placeholder pod. The `taskRun` pods on a `cordoned` node continues running +until completion. The deletion of a placeholder pod triggers creating a new placeholder pod on any available node +such that the rest of the `pipelineRun` can continue without any disruption until it finishes. + #### Specifying `Workspaces` in `PipelineRuns` For a `PipelineRun` to execute a `Pipeline` that includes one or more `Workspaces`, it needs to diff --git a/pkg/reconciler/pipelinerun/affinity_assistant.go b/pkg/reconciler/pipelinerun/affinity_assistant.go index edcb9c800b5..7a27bdda4b2 100644 --- a/pkg/reconciler/pipelinerun/affinity_assistant.go +++ b/pkg/reconciler/pipelinerun/affinity_assistant.go @@ -33,32 +33,35 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" errorutils "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/kmeta" "knative.dev/pkg/logging" ) const ( - // ReasonCouldntCreateAffinityAssistantStatefulSet indicates that a PipelineRun uses workspaces with PersistentVolumeClaim + // ReasonCouldntCreateOrUpdateAffinityAssistantStatefulSet indicates that a PipelineRun uses workspaces with PersistentVolumeClaim // as a volume source and expect an Assistant StatefulSet, but couldn't create a StatefulSet. - ReasonCouldntCreateAffinityAssistantStatefulSet = "CouldntCreateAffinityAssistantStatefulSet" + ReasonCouldntCreateOrUpdateAffinityAssistantStatefulSet = "CouldntCreateOrUpdateAffinityAssistantstatefulSet" featureFlagDisableAffinityAssistantKey = "disable-affinity-assistant" ) -// createAffinityAssistants creates an Affinity Assistant StatefulSet for every workspace in the PipelineRun that +// createOrUpdateAffinityAssistants creates an Affinity Assistant StatefulSet for every workspace in the PipelineRun that // use a PersistentVolumeClaim volume. This is done to achieve Node Affinity for all TaskRuns that // share the workspace volume and make it possible for the tasks to execute parallel while sharing volume. -func (c *Reconciler) createAffinityAssistants(ctx context.Context, wb []v1beta1.WorkspaceBinding, pr *v1beta1.PipelineRun, namespace string) error { +func (c *Reconciler) createOrUpdateAffinityAssistants(ctx context.Context, wb []v1beta1.WorkspaceBinding, pr *v1beta1.PipelineRun, namespace string) error { logger := logging.FromContext(ctx) cfg := config.FromContextOrDefaults(ctx) var errs []error + var unschedulableNodes sets.Set[string] = nil for _, w := range wb { if w.PersistentVolumeClaim != nil || w.VolumeClaimTemplate != nil { affinityAssistantName := getAffinityAssistantName(w.Name, pr.Name) - _, err := c.KubeClientSet.AppsV1().StatefulSets(namespace).Get(ctx, affinityAssistantName, metav1.GetOptions{}) + a, err := c.KubeClientSet.AppsV1().StatefulSets(namespace).Get(ctx, affinityAssistantName, metav1.GetOptions{}) claimName := getClaimName(w, *kmeta.NewControllerRef(pr)) switch { + // check whether the affinity assistant (StatefulSet) exists or not, create one if it does not exist case apierrors.IsNotFound(err): affinityAssistantStatefulSet := affinityAssistantStatefulSet(affinityAssistantName, pr, claimName, c.Images.NopImage, cfg.Defaults.DefaultAAPodTemplate) _, err := c.KubeClientSet.AppsV1().StatefulSets(namespace).Create(ctx, affinityAssistantStatefulSet, metav1.CreateOptions{}) @@ -68,6 +71,40 @@ func (c *Reconciler) createAffinityAssistants(ctx context.Context, wb []v1beta1. if err == nil { logger.Infof("Created StatefulSet %s in namespace %s", affinityAssistantName, namespace) } + // check whether the affinity assistant (StatefulSet) exists and the affinity assistant pod is created + // this check requires the StatefulSet to have the readyReplicas set to 1 to allow for any delay between the StatefulSet creation + // and the necessary pod creation, the delay can be caused by any dependency on PVCs and PVs creation + // this case addresses issues specified in https://github.com/tektoncd/pipeline/issues/6586 + case err == nil && a != nil && a.Status.ReadyReplicas == 1: + if unschedulableNodes == nil { + ns, err := c.KubeClientSet.CoreV1().Nodes().List(ctx, metav1.ListOptions{ + FieldSelector: "spec.unschedulable=true", + }) + if err != nil { + errs = append(errs, fmt.Errorf("could not get the list of nodes, err: %w", err)) + } + unschedulableNodes = sets.Set[string]{} + // maintain the list of nodes which are unschedulable + for _, n := range ns.Items { + unschedulableNodes.Insert(n.Name) + } + } + if unschedulableNodes.Len() > 0 { + // get the pod created for a given StatefulSet, pod is assigned ordinal of 0 with the replicas set to 1 + p, err := c.KubeClientSet.CoreV1().Pods(pr.Namespace).Get(ctx, a.Name+"-0", metav1.GetOptions{}) + // ignore instead of failing if the affinity assistant pod was not found + if err != nil && !apierrors.IsNotFound(err) { + errs = append(errs, fmt.Errorf("could not get the affinity assistant pod for StatefulSet %s: %w", a.Name, err)) + } + // check the node which hosts the affinity assistant pod if it is unschedulable or cordoned + if p != nil && unschedulableNodes.Has(p.Spec.NodeName) { + // if the node is unschedulable, delete the affinity assistant pod such that a StatefulSet can recreate the same pod on a different node + err = c.KubeClientSet.CoreV1().Pods(p.Namespace).Delete(ctx, p.Name, metav1.DeleteOptions{}) + if err != nil { + errs = append(errs, fmt.Errorf("error deleting affinity assistant pod %s in ns %s: %w", p.Name, p.Namespace, err)) + } + } + } case err != nil: errs = append(errs, fmt.Errorf("failed to retrieve StatefulSet %s: %w", affinityAssistantName, err)) } @@ -107,7 +144,7 @@ func (c *Reconciler) cleanupAffinityAssistants(ctx context.Context, pr *v1beta1. 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]) + return fmt.Sprintf("%s-%s", workspace.ComponentNameAffinityAssistant, hashString[:10]) } func getStatefulSetLabels(pr *v1beta1.PipelineRun, affinityAssistantName string) map[string]string { diff --git a/pkg/reconciler/pipelinerun/affinity_assistant_test.go b/pkg/reconciler/pipelinerun/affinity_assistant_test.go index ff7fb6fad98..2e347a66bf7 100644 --- a/pkg/reconciler/pipelinerun/affinity_assistant_test.go +++ b/pkg/reconciler/pipelinerun/affinity_assistant_test.go @@ -18,6 +18,7 @@ package pipelinerun import ( "context" + "errors" "testing" "github.com/google/go-cmp/cmp" @@ -28,15 +29,36 @@ import ( "github.com/tektoncd/pipeline/pkg/workspace" "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/parse" + v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" fakek8s "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/kubernetes/typed/core/v1/fake" + testing2 "k8s.io/client-go/testing" logtesting "knative.dev/pkg/logging/testing" "knative.dev/pkg/system" _ "knative.dev/pkg/system/testing" // Setup system.Namespace() ) +var workspaceName = "test-workspace" + +var testPipelineRun = &v1beta1.PipelineRun{ + TypeMeta: metav1.TypeMeta{Kind: "PipelineRun"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pipelinerun", + }, + Spec: v1beta1.PipelineRunSpec{ + Workspaces: []v1beta1.WorkspaceBinding{{ + Name: workspaceName, + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + }, + }}, + }, +} + // TestCreateAndDeleteOfAffinityAssistant tests to create and delete an Affinity Assistant // for a given PipelineRun with a PVC workspace func TestCreateAndDeleteOfAffinityAssistant(t *testing.T) { @@ -49,26 +71,9 @@ func TestCreateAndDeleteOfAffinityAssistant(t *testing.T) { Images: pipeline.Images{}, } - workspaceName := "testws" - pipelineRunName := "pipelinerun-1" - testPipelineRun := &v1beta1.PipelineRun{ - TypeMeta: metav1.TypeMeta{Kind: "PipelineRun"}, - ObjectMeta: metav1.ObjectMeta{ - Name: pipelineRunName, - }, - Spec: v1beta1.PipelineRunSpec{ - Workspaces: []v1beta1.WorkspaceBinding{{ - Name: workspaceName, - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: "myclaim", - }, - }}, - }, - } - - err := c.createAffinityAssistants(ctx, testPipelineRun.Spec.Workspaces, testPipelineRun, testPipelineRun.Namespace) + err := c.createOrUpdateAffinityAssistants(ctx, testPipelineRun.Spec.Workspaces, testPipelineRun, testPipelineRun.Namespace) if err != nil { - t.Errorf("unexpected error from createAffinityAssistants: %v", err) + t.Errorf("unexpected error from createOrUpdateAffinityAssistants: %v", err) } expectedAffinityAssistantName := getAffinityAssistantName(workspaceName, testPipelineRun.Name) @@ -88,6 +93,128 @@ func TestCreateAndDeleteOfAffinityAssistant(t *testing.T) { } } +// TestCreateAffinityAssistantWhenNodeIsCordoned tests an existing Affinity Assistant can identify the node failure and +// can migrate the affinity assistant pod to a healthy node so that the existing pipelineRun runs to competition +func TestCreateOrUpdateAffinityAssistantWhenNodeIsCordoned(t *testing.T) { + expectedAffinityAssistantName := getAffinityAssistantName(workspaceName, testPipelineRun.Name) + + aa := []*v1.StatefulSet{{ + TypeMeta: metav1.TypeMeta{ + Kind: "StatefulSet", + APIVersion: "apps/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: expectedAffinityAssistantName, + Labels: getStatefulSetLabels(testPipelineRun, expectedAffinityAssistantName), + }, + Status: v1.StatefulSetStatus{ + ReadyReplicas: 1, + }, + }} + + nodes := []*corev1.Node{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "soon-to-be-cordoned-node", + }, + Spec: corev1.NodeSpec{ + Unschedulable: true, + }, + }} + + p := []*corev1.Pod{{ + ObjectMeta: metav1.ObjectMeta{ + Name: expectedAffinityAssistantName + "-0", + }, + Spec: corev1.PodSpec{ + NodeName: "soon-to-be-cordoned-node", + }, + }} + + tests := []struct { + name, verb, resource string + data Data + validatePodDeletion, expectedError bool + }{{ + name: "createOrUpdateAffinityAssistants must ignore missing affinity assistant pod, this could be interim and must not fail the entire pipelineRun", + data: Data{ + StatefulSets: aa, + Nodes: nodes, + }, + }, { + name: "createOrUpdateAffinityAssistants must delete an affinity assistant pod since the node on which its scheduled is marked as unschedulable", + data: Data{ + StatefulSets: aa, + Nodes: nodes, + Pods: p, + }, + validatePodDeletion: true, + }, { + name: "createOrUpdateAffinityAssistants must catch an error while listing nodes", + data: Data{ + StatefulSets: aa, + Nodes: nodes, + }, + verb: "list", + resource: "nodes", + expectedError: true, + }, { + name: "createOrUpdateAffinityAssistants must catch an error while getting pods", + data: Data{ + StatefulSets: aa, + Nodes: nodes, + }, + verb: "get", + resource: "pods", + expectedError: true, + }, { + name: "createOrUpdateAffinityAssistants must catch an error while deleting pods", + data: Data{ + StatefulSets: aa, + Nodes: nodes, + Pods: p, + }, + verb: "delete", + resource: "pods", + expectedError: true, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, c, cancel := seedTestData(tt.data) + defer cancel() + + if tt.resource == "nodes" { + // introduce a reactor to mock node error + c.KubeClientSet.CoreV1().(*fake.FakeCoreV1).PrependReactor(tt.verb, tt.resource, + func(action testing2.Action) (handled bool, ret runtime.Object, err error) { + return true, &corev1.NodeList{}, errors.New("error listing nodes") + }) + } + if tt.resource == "pods" { + // introduce a reactor to mock pod error + c.KubeClientSet.CoreV1().(*fake.FakeCoreV1).PrependReactor(tt.verb, tt.resource, + func(action testing2.Action) (handled bool, ret runtime.Object, err error) { + return true, &corev1.Pod{}, errors.New("error listing/deleting pod") + }) + } + + err := c.createOrUpdateAffinityAssistants(ctx, testPipelineRun.Spec.Workspaces, testPipelineRun, testPipelineRun.Namespace) + if !tt.expectedError && err != nil { + t.Errorf("expected no error from createOrUpdateAffinityAssistants for the test \"%s\", but got: %v", tt.name, err) + } + // the affinity assistant pod must have been deleted when it was running on a cordoned node + if tt.validatePodDeletion { + _, err = c.KubeClientSet.CoreV1().Pods(testPipelineRun.Namespace).Get(ctx, expectedAffinityAssistantName+"-0", metav1.GetOptions{}) + if !apierrors.IsNotFound(err) { + t.Errorf("expected a NotFound response, got: %v", err) + } + } + if tt.expectedError && err == nil { + t.Errorf("expected error from createOrUpdateAffinityAssistants, but got no error") + } + }) + } +} + func TestPipelineRunPodTemplatesArePropagatedToAffinityAssistant(t *testing.T) { prWithCustomPodTemplate := &v1beta1.PipelineRun{ TypeMeta: metav1.TypeMeta{Kind: "PipelineRun"}, @@ -284,21 +411,6 @@ func TestThatAffinityAssistantNameIsNoLongerThan53(t *testing.T) { // cleanup of Affinity Assistants is omitted when the // Affinity Assistant is disabled func TestThatCleanupIsAvoidedIfAssistantIsDisabled(t *testing.T) { - testPipelineRun := &v1beta1.PipelineRun{ - TypeMeta: metav1.TypeMeta{Kind: "PipelineRun"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pipelinerun", - }, - Spec: v1beta1.PipelineRunSpec{ - Workspaces: []v1beta1.WorkspaceBinding{{ - Name: "test-workspace", - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: "myclaim", - }, - }}, - }, - } - configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, Data: map[string]string{ @@ -514,3 +626,28 @@ spec: }) } } + +type Data struct { + StatefulSets []*v1.StatefulSet + Nodes []*corev1.Node + Pods []*corev1.Pod +} + +func seedTestData(d Data) (context.Context, Reconciler, func()) { + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + + c := Reconciler{ + KubeClientSet: fakek8s.NewSimpleClientset(), + } + for _, s := range d.StatefulSets { + c.KubeClientSet.AppsV1().StatefulSets(s.Namespace).Create(ctx, s, metav1.CreateOptions{}) + } + for _, n := range d.Nodes { + c.KubeClientSet.CoreV1().Nodes().Create(ctx, n, metav1.CreateOptions{}) + } + for _, p := range d.Pods { + c.KubeClientSet.CoreV1().Pods(p.Namespace).Create(ctx, p, metav1.CreateOptions{}) + } + return ctx, c, cancel +} diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index e2d103fe392..23068a99cee 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -609,16 +609,18 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get return controller.NewPermanentError(err) } } + } - if !c.isAffinityAssistantDisabled(ctx) { - // create Affinity Assistant (StatefulSet) so that taskRun pods that share workspace PVC achieve Node Affinity - if err = c.createAffinityAssistants(ctx, pr.Spec.Workspaces, pr, pr.Namespace); err != nil { - logger.Errorf("Failed to create affinity assistant StatefulSet for PipelineRun %s: %v", pr.Name, err) - pr.Status.MarkFailed(ReasonCouldntCreateAffinityAssistantStatefulSet, - "Failed to create StatefulSet for PipelineRun %s/%s correctly: %s", - pr.Namespace, pr.Name, err) - return controller.NewPermanentError(err) - } + // Make an attempt to create Affinity Assistant if it does not exist + // if the Affinity Assistant already exists, handle the possibility of assigned node becoming unschedulable by deleting the pod + if !c.isAffinityAssistantDisabled(ctx) { + // create Affinity Assistant (StatefulSet) so that taskRun pods that share workspace PVC achieve Node Affinity + if err = c.createOrUpdateAffinityAssistants(ctx, pr.Spec.Workspaces, pr, pr.Namespace); err != nil { + logger.Errorf("Failed to create affinity assistant StatefulSet for PipelineRun %s: %v", pr.Name, err) + pr.Status.MarkFailed(ReasonCouldntCreateOrUpdateAffinityAssistantStatefulSet, + "Failed to create StatefulSet or update affinity assistant replicas for PipelineRun %s/%s correctly: %s", + pr.Namespace, pr.Name, err) + return controller.NewPermanentError(err) } }