From 617cc412af408f3f417bb180d7bd620bf0de86be Mon Sep 17 00:00:00 2001 From: Quan Zhang Date: Wed, 7 Jun 2023 15:32:55 -0400 Subject: [PATCH] TEP-0135: Introduce coscheduling feature flags Part of [#6740][#6740]. [TEP-0135][tep-0135] introduces a feature that allows a cluster operator to ensure that all of a PipelineRun's pods are scheduled to the same node. This commit introduces a new feature flag `coscheduling` which works together with the `disable-affinity-assistant` feature flag to determine the Affinity Assistant behavior. The usage of the new feature flag will be added in the follow-up PRs. The details of the `coscheduling` feature flag can be found in the [Configuration][configuration] section of TEP-0135. The details of the `disable-affinity-assistant` feature flag can be found in the [Upgrade and Migration Strategy][strategy] section of TEP-0135. NOTE: this feature is WIP, please do not use on this feature. /kind feature [#6740]: https://github.com/tektoncd/pipeline/issues/6740 [tep-0135]: https://github.com/tektoncd/community/blob/main/teps/0135-coscheduling-pipelinerun-pods.md [configuration]: https://github.com/tektoncd/community/blob/main/teps/0135-coscheduling-pipelinerun-pods.md#configuration [strategy]: https://github.com/tektoncd/community/blob/main/teps/0135-coscheduling-pipelinerun-pods.md#configuration --- config/config-feature-flags.yaml | 11 ++++ pkg/apis/config/feature_flags.go | 39 +++++++++++- pkg/apis/config/feature_flags_test.go | 14 +++++ ...-coscheduling-affinity-assistant-comb.yaml | 23 +++++++ .../feature-flags-invalid-coscheduling.yaml | 22 +++++++ pkg/apis/config/testing/featureflags.go | 40 ++++++++++++ .../affinityassistant_types.go | 40 ++++++++++++ .../affinityassistant_types_test.go | 63 +++++++++++++++++++ 8 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 pkg/apis/config/testdata/feature-flags-invalid-coscheduling-affinity-assistant-comb.yaml create mode 100644 pkg/apis/config/testdata/feature-flags-invalid-coscheduling.yaml create mode 100644 pkg/apis/config/testing/featureflags.go create mode 100644 pkg/internal/affinityassistant/affinityassistant_types.go create mode 100644 pkg/internal/affinityassistant/affinityassistant_types_test.go diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index 4046dafd8b3..d207c44c9ec 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -30,6 +30,17 @@ data: # https://github.com/tektoncd/pipeline/blob/main/docs/workspaces.md#affinity-assistant-and-specifying-workspace-order-in-a-pipeline # or https://github.com/tektoncd/pipeline/pull/2630 for more info. disable-affinity-assistant: "false" + # Setting this flag will determine how PipelineRun Pods are scheduled with Affinity Assistant. + # Acceptable values are "disabled" (default), "coschedule-workspaces", "coschedule-pipelineruns", or "isolate-pipelineruns". + # + # See more in the "Configuration" section of tep-0135: coscheduling-pipelinerun-pods for more info about the options: + # https://github.com/tektoncd/community/blob/main/teps/0135-coscheduling-pipelinerun-pods.md#configuration + # See more in the "Upgrade and Migration Strategy" section of tep-0135: coscheduling-pipelinerun-pods for more info about + # migration strategy against the "disable-affinity-assistant" field: + # https://github.com/tektoncd/community/blob/main/teps/0135-coscheduling-pipelinerun-pods.md#upgrade-and-migration-strategy + + # NOTE: this feature flag is WIP and DO NOT use this feature or change the value. + coscheduling: "disabled" # Setting this flag to "true" will prevent Tekton scanning attached # service accounts and injecting any credentials it finds into your # Steps. diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index 3b5350db346..db338840f5d 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -42,6 +42,14 @@ const ( // IgnoreNoMatchPolicy is the value used for "trusted-resources-verification-no-match-policy" to skip verification // when no matching policies are found IgnoreNoMatchPolicy = "ignore" + // CosheduleWorkspaces is the value used for "coscheduling" to coschedule PipelineRun Pods sharing the same PVC workspaces to the same node + CoscheduleWorkspaces = "coschedule-workspaces" + // CoshedulePipelineRuns is the value used for "coscheduling" to coschedule all PipelineRun Pods to the same node + CoschedulePipelineRuns = "coschedule-pipelineruns" + // CoscheduleIsolatePipelineRuns is the value used for "coscheduling" to coschedule all PipelineRun Pods to the same node, and only allows one PipelineRun to run on a node at a time + CoscheduleIsolatePipelineRuns = "isolate-pipelineruns" + // CoscheduleDisabled is the value used for "coscheduling" to disabled PipelineRun Pods coscheduling + CoscheduleDisabled = "disabled" // ResultExtractionMethodTerminationMessage is the value used for "results-from" as a way to extract results from tasks using kubernetes termination message. ResultExtractionMethodTerminationMessage = "termination-message" // ResultExtractionMethodSidecarLogs is the value used for "results-from" as a way to extract results from tasks using sidecar logs. @@ -76,6 +84,8 @@ const ( DefaultResultExtractionMethod = ResultExtractionMethodTerminationMessage // DefaultMaxResultSize is the default value in bytes for the size of a result DefaultMaxResultSize = 4096 + // DefaultCoscheduling is the default value for coscheduling + DefaultCoscheduling = CoscheduleDisabled disableAffinityAssistantKey = "disable-affinity-assistant" disableCredsInitKey = "disable-creds-init" @@ -90,6 +100,7 @@ const ( enableProvenanceInStatus = "enable-provenance-in-status" resultExtractionMethod = "results-from" maxResultSize = "max-result-size" + coschedulingKey = "coscheduling" ) // DefaultFeatureFlags holds all the default configurations for the feature flags configmap. @@ -119,6 +130,7 @@ type FeatureFlags struct { EnableProvenanceInStatus bool ResultExtractionMethod string MaxResultSize int + Coscheduling string } // GetFeatureFlagsConfigName returns the name of the configmap containing all @@ -182,7 +194,9 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setEnforceNonFalsifiability(cfgMap, tc.EnableAPIFields, &tc.EnforceNonfalsifiability); err != nil { return nil, err } - + if err := setCoscheduling(cfgMap, DefaultCoscheduling, tc.DisableAffinityAssistant, &tc.Coscheduling); err != nil { + return nil, err + } // Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if // enable-api-fields is "alpha". If enable-api-fields is not "alpha" then fall back to the value of // each feature's individual flag. @@ -215,6 +229,29 @@ func setEnabledAPIFields(cfgMap map[string]string, defaultValue string, feature return nil } +// setCoscheduling sets the "coscheduling" flag based on the content of a given map. +// If the feature gate is invalid or incompatible with `disable-affinity-assistant`, then an error is returned. +func setCoscheduling(cfgMap map[string]string, defaultValue string, disabledAffinityAssistant bool, feature *string) error { + value := defaultValue + if cfg, ok := cfgMap[coschedulingKey]; ok { + value = strings.ToLower(cfg) + } + + switch value { + case CoscheduleDisabled, CoscheduleWorkspaces, CoschedulePipelineRuns, CoscheduleIsolatePipelineRuns: + // validate that "coscheduling" is compatible with "disable-affinity-assistant" + // "coscheduling" must be set to "disabled" when "disable-affinity-assistant" is false + if !disabledAffinityAssistant && value != CoscheduleDisabled { + return fmt.Errorf("coscheduling value %v is incompatible with %v setting to false", value, disableAffinityAssistantKey) + } + *feature = value + default: + return fmt.Errorf("invalid value for feature flag %q: %q", coschedulingKey, value) + } + + return nil +} + // setEnforceNonFalsifiability sets the "enforce-nonfalsifiability" flag based on the content of a given map. // If the feature gate is invalid, then an error is returned. func setEnforceNonFalsifiability(cfgMap map[string]string, enableAPIFields string, feature *string) error { diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index 10cfa1150ea..d6350353204 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -51,6 +51,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, + Coscheduling: config.DefaultCoscheduling, }, fileName: config.GetFeatureFlagsConfigName(), }, @@ -67,6 +68,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { VerificationNoMatchPolicy: config.FailNoMatchPolicy, EnableProvenanceInStatus: false, ResultExtractionMethod: "termination-message", + Coscheduling: config.CoscheduleDisabled, MaxResultSize: 4096, }, @@ -89,6 +91,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, + Coscheduling: config.DefaultCoscheduling, }, fileName: "feature-flags-enable-api-fields-overrides-bundles-and-custom-tasks", }, @@ -107,6 +110,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, + Coscheduling: config.DefaultCoscheduling, }, fileName: "feature-flags-bundles-and-custom-tasks", }, @@ -125,6 +129,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, + Coscheduling: config.DefaultCoscheduling, }, fileName: "feature-flags-beta-api-fields", }, @@ -139,6 +144,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, + Coscheduling: config.DefaultCoscheduling, }, fileName: "feature-flags-enforce-nonfalsifiability-spire", }, @@ -151,6 +157,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, ResultExtractionMethod: config.ResultExtractionMethodSidecarLogs, MaxResultSize: 8192, + Coscheduling: config.DefaultCoscheduling, }, fileName: "feature-flags-results-via-sidecar-logs", }, @@ -181,6 +188,7 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) { EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, + Coscheduling: config.DefaultCoscheduling, } verifyConfigFileWithExpectedFeatureFlagsConfig(t, FeatureFlagsConfigEmptyName, expectedConfig) } @@ -240,6 +248,12 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) { }, { fileName: "feature-flags-spire-with-stable", want: `"enforce-nonfalsifiability" can be set to non-default values ("spire") only in alpha`, + }, { + fileName: "feature-flags-invalid-coscheduling-affinity-assistant-comb", + want: `coscheduling value coschedule-workspaces is incompatible with disable-affinity-assistant setting to false`, + }, { + fileName: "feature-flags-invalid-coscheduling", + want: `invalid value for feature flag "coscheduling": "invalid"`, }} { t.Run(tc.fileName, func(t *testing.T) { cm := test.ConfigMapFromTestFile(t, tc.fileName) diff --git a/pkg/apis/config/testdata/feature-flags-invalid-coscheduling-affinity-assistant-comb.yaml b/pkg/apis/config/testdata/feature-flags-invalid-coscheduling-affinity-assistant-comb.yaml new file mode 100644 index 00000000000..5cf74bdcfd6 --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags-invalid-coscheduling-affinity-assistant-comb.yaml @@ -0,0 +1,23 @@ + +# Copyright 2023 The Tekton Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: feature-flags + namespace: tekton-pipelines +data: + coscheduling: "coschedule-workspaces" + disable-affinity-assistant: "false" diff --git a/pkg/apis/config/testdata/feature-flags-invalid-coscheduling.yaml b/pkg/apis/config/testdata/feature-flags-invalid-coscheduling.yaml new file mode 100644 index 00000000000..71dcf26f7c6 --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags-invalid-coscheduling.yaml @@ -0,0 +1,22 @@ + +# Copyright 2023 The Tekton Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: feature-flags + namespace: tekton-pipelines +data: + coscheduling: "invalid" diff --git a/pkg/apis/config/testing/featureflags.go b/pkg/apis/config/testing/featureflags.go new file mode 100644 index 00000000000..9a1885b1fc4 --- /dev/null +++ b/pkg/apis/config/testing/featureflags.go @@ -0,0 +1,40 @@ +/* +Copyright 2023 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testing + +import ( + "context" + "testing" + + "github.com/tektoncd/pipeline/pkg/apis/config" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/logging" +) + +// SetFeatureFlags sets the feature-flags ConfigMap values in an existing context (for use in testing) +func SetFeatureFlags(ctx context.Context, t *testing.T, data map[string]string) context.Context { + t.Helper() + s := config.NewStore(logging.FromContext(ctx).Named("config-store")) + s.OnConfigChanged(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: config.GetFeatureFlagsConfigName(), + }, + Data: data, + }) + return s.ToContext(ctx) +} diff --git a/pkg/internal/affinityassistant/affinityassistant_types.go b/pkg/internal/affinityassistant/affinityassistant_types.go new file mode 100644 index 00000000000..c4d0e406045 --- /dev/null +++ b/pkg/internal/affinityassistant/affinityassistant_types.go @@ -0,0 +1,40 @@ +package affinityassistant + +import ( + "context" + + "github.com/tektoncd/pipeline/pkg/apis/config" +) + +type AffinityAssitantBehavior string + +const ( + AffinityAssistantDisabled = AffinityAssitantBehavior("AffinityAssistantDisabled") + AffinityAssistantPerWorkspace = AffinityAssitantBehavior("AffinityAssistantPerWorkspace") + AffinityAssistantPerPipelineRun = AffinityAssitantBehavior("AffinityAssistantPerPipelineRun") + AffinityAssistantPerPipelineRunWithIsolation = AffinityAssitantBehavior("AffinityAssistantPerPipelineRunWithIsolation") +) + +// GetAffinityAssistantBehavior returns an AffinityAssitantBehavior based on the +// combination of "disable-affinity-assistant" and "coscheduling" feature flags +// TODO(#6740)(WIP): consume this function in the PipelineRun reconciler to determine Affinity Assistant behavior. +func GetAffinityAssistantBehavior(ctx context.Context) AffinityAssitantBehavior { + cfg := config.FromContextOrDefaults(ctx) + + if !cfg.FeatureFlags.DisableAffinityAssistant { + return AffinityAssistantPerWorkspace + } + + switch cfg.FeatureFlags.Coscheduling { + case config.CoscheduleWorkspaces: + return AffinityAssistantPerWorkspace + case config.CoschedulePipelineRuns: + return AffinityAssistantPerPipelineRun + case config.CoscheduleIsolatePipelineRuns: + return AffinityAssistantPerPipelineRunWithIsolation + case config.CoscheduleDisabled: + return AffinityAssistantDisabled + } + + return "" +} diff --git a/pkg/internal/affinityassistant/affinityassistant_types_test.go b/pkg/internal/affinityassistant/affinityassistant_types_test.go new file mode 100644 index 00000000000..fa7bc2d58e5 --- /dev/null +++ b/pkg/internal/affinityassistant/affinityassistant_types_test.go @@ -0,0 +1,63 @@ +package affinityassistant + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" + "github.com/tektoncd/pipeline/test/diff" + + "github.com/tektoncd/pipeline/pkg/apis/config" +) + +func Test_GetAffinityAssistantBehavior(t *testing.T) { + tcs := []struct { + name string + configMap map[string]string + expect AffinityAssitantBehavior + }{{ + name: "affinity-assistant-enabled", + configMap: map[string]string{ + "disable-affinity-assistant": "false", + }, + expect: AffinityAssistantPerWorkspace, + }, { + name: "affinity-assistant-disabled-coscheduling-workspaces", + configMap: map[string]string{ + "disable-affinity-assistant": "true", + "coscheduling": config.CoscheduleWorkspaces, + }, + expect: AffinityAssistantPerWorkspace, + }, { + name: "affinity-assistant-disabled-coscheduling-pipelineruns", + configMap: map[string]string{ + "disable-affinity-assistant": "true", + "coscheduling": config.CoschedulePipelineRuns, + }, + expect: AffinityAssistantPerPipelineRun, + }, { + name: "affinity-assistant-disabled-coscheduling-isolate-pipelineruns", + configMap: map[string]string{ + "disable-affinity-assistant": "true", + "coscheduling": config.CoscheduleIsolatePipelineRuns, + }, + expect: AffinityAssistantPerPipelineRunWithIsolation, + }, { + name: "affinity-assistant-disabled-coscheduling-disabled", + configMap: map[string]string{ + "disable-affinity-assistant": "true", + "coscheduling": config.CoscheduleDisabled, + }, + expect: AffinityAssistantDisabled, + }} + + for _, tc := range tcs { + ctx := cfgtesting.SetFeatureFlags(context.Background(), t, tc.configMap) + get := GetAffinityAssistantBehavior(ctx) + + if d := cmp.Diff(tc.expect, get); d != "" { + t.Errorf("AffinityAssitantBehavior mismatch: %v", diff.PrintWantGot(d)) + } + } +}