From 536f2598684e2f80c39d14bbc83fb27987743ca2 Mon Sep 17 00:00:00 2001 From: Heba <31887807+helayoty@users.noreply.github.com> Date: Wed, 1 May 2024 15:40:44 -0700 Subject: [PATCH 1/2] feat: Add Karpenter Azure cloud provider NodePool (#369) **Reason for Change**: Add an example for `Karpenter` cloud provider `NodePool` for Azure. **Requirements** - [ ] added unit tests and e2e tests (if applicable). **Issue Fixed**: **Notes for Reviewers**: Signed-off-by: Heba Elayoty --- examples/kaito-karpenter-noodepool.yaml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 examples/kaito-karpenter-noodepool.yaml diff --git a/examples/kaito-karpenter-noodepool.yaml b/examples/kaito-karpenter-noodepool.yaml new file mode 100644 index 000000000..676fbdb6e --- /dev/null +++ b/examples/kaito-karpenter-noodepool.yaml @@ -0,0 +1,22 @@ +--- +apiVersion: karpenter.sh/v1beta1 +kind: NodePool +metadata: + name: kaito-nodepool + annotations: + kubernetes.io/description: "General purpose NodePool for generic workloads" +spec: + disruption: + consolidateAfter: Never + consolidationPolicy: WhenEmpty + expireAfter: Never + template: + spec: + requirements: +--- +apiVersion: karpenter.azure.com/v1alpha2 +kind: AKSNodeClass +metadata: + name: default +spec: + imageFamily: Ubuntu2204 From a4bcec4078a9a0b1bef3bf077e7cad07daf893da Mon Sep 17 00:00:00 2001 From: Heba <31887807+helayoty@users.noreply.github.com> Date: Wed, 1 May 2024 16:06:18 -0700 Subject: [PATCH 2/2] feat: Add feature gates flag (#368) **Reason for Change**: - Add `feature-gates` flag to the Kaito controller. - Support the `karpenter` feature gate with default value, `false`. - Enable setting the feature gate flag via helm chart. In addition to some chores: - Exclude the auto generated and mock files from unit test coverage. - Upgrade k8s version using in the tests to the current supported version. **Requirements** - [x] added unit tests and e2e tests (if applicable). **Issue Fixed**: **Notes for Reviewers**: --------- Signed-off-by: Heba Elayoty --- .github/workflows/e2e-workflow.yml | 2 +- Makefile | 6 +- api/v1alpha1/workspace_validation_test.go | 6 +- .../kaito/workspace/templates/deployment.yaml | 2 + charts/kaito/workspace/values.yaml | 2 + cmd/main.go | 8 +++ pkg/controllers/workspace_controller.go | 5 +- pkg/controllers/workspace_gc_finalizer.go | 4 +- pkg/featuregates/featuregates.go | 47 ++++++++++++++ pkg/featuregates/featuregates_test.go | 63 +++++++++++++++++++ pkg/tuning/preset-tuning_test.go | 5 +- pkg/utils/common.go | 11 ++-- pkg/utils/consts/consts.go | 11 ++++ 13 files changed, 153 insertions(+), 19 deletions(-) create mode 100644 pkg/featuregates/featuregates.go create mode 100644 pkg/featuregates/featuregates_test.go create mode 100644 pkg/utils/consts/consts.go diff --git a/.github/workflows/e2e-workflow.yml b/.github/workflows/e2e-workflow.yml index a271fa0c5..e31021b5c 100644 --- a/.github/workflows/e2e-workflow.yml +++ b/.github/workflows/e2e-workflow.yml @@ -19,7 +19,7 @@ on: default: "eastus" k8s_version: type: string - default: "1.27" + default: "1.29.2" secrets: E2E_CLIENT_ID: required: true diff --git a/Makefile b/Makefile index 339dab4f0..d10bfcb25 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ GINKGO := $(TOOLS_BIN_DIR)/$(GINKGO_BIN)-$(GINKGO_VER) AZURE_SUBSCRIPTION_ID ?= $(AZURE_SUBSCRIPTION_ID) AZURE_LOCATION ?= eastus -AKS_K8S_VERSION ?= 1.27.2 +AKS_K8S_VERSION ?= 1.29.2 AZURE_RESOURCE_GROUP ?= demo AZURE_CLUSTER_NAME ?= kaito-demo AZURE_RESOURCE_GROUP_MC=MC_$(AZURE_RESOURCE_GROUP)_$(AZURE_CLUSTER_NAME)_$(AZURE_LOCATION) @@ -84,7 +84,9 @@ fmt: ## Run go fmt against code. ## -------------------------------------- .PHONY: unit-test unit-test: ## Run unit tests. - go test -v $(shell go list ./pkg/... ./api/... | grep -v /vendor) -race -coverprofile=coverage.txt -covermode=atomic + go test -v $(shell go list ./pkg/... ./api/... | \ + grep -v -e /vendor -e /api/v1alpha1/zz_generated.deepcopy.go -e /pkg/utils/test/...) \ + -race -coverprofile=coverage.txt -covermode=atomic go tool cover -func=coverage.txt inference-api-e2e: diff --git a/api/v1alpha1/workspace_validation_test.go b/api/v1alpha1/workspace_validation_test.go index 6c44ff1cf..65d667009 100644 --- a/api/v1alpha1/workspace_validation_test.go +++ b/api/v1alpha1/workspace_validation_test.go @@ -12,7 +12,7 @@ import ( "testing" "github.com/azure/kaito/pkg/k8sclient" - "github.com/azure/kaito/pkg/utils" + "github.com/azure/kaito/pkg/utils/consts" "github.com/azure/kaito/pkg/utils/plugin" "k8s.io/apimachinery/pkg/runtime" @@ -694,8 +694,8 @@ func TestWorkspaceValidateUpdate(t *testing.T) { func TestTuningSpecValidateCreate(t *testing.T) { RegisterValidationTestModels() // Set ReleaseNamespace Env - os.Setenv(utils.DefaultReleaseNamespaceEnvVar, DefaultReleaseNamespace) - defer os.Unsetenv(utils.DefaultReleaseNamespaceEnvVar) + os.Setenv(consts.DefaultReleaseNamespaceEnvVar, DefaultReleaseNamespace) + defer os.Unsetenv(consts.DefaultReleaseNamespaceEnvVar) // Create fake client with default ConfigMap scheme := runtime.NewScheme() diff --git a/charts/kaito/workspace/templates/deployment.yaml b/charts/kaito/workspace/templates/deployment.yaml index 53d7e4a79..b3a3b0d8e 100644 --- a/charts/kaito/workspace/templates/deployment.yaml +++ b/charts/kaito/workspace/templates/deployment.yaml @@ -32,6 +32,8 @@ spec: {{- toYaml .Values.securityContext | nindent 12 }} image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" imagePullPolicy: {{ .Values.image.pullPolicy }} + args: + - --feature-gates={{- range $k, $v := .Values.featureGates }}{{ $k }}={{ $v}}{{- end }} env: - name: WEBHOOK_SERVICE value: {{ include "kaito.fullname" . }} diff --git a/charts/kaito/workspace/values.yaml b/charts/kaito/workspace/values.yaml index 3841f064c..5dd4ea2e6 100644 --- a/charts/kaito/workspace/values.yaml +++ b/charts/kaito/workspace/values.yaml @@ -15,6 +15,8 @@ securityContext: capabilities: drop: - "ALL" +featureGates: + Karpenter: "false" webhook: port: 9443 presetRegistryName: mcr.microsoft.com/aks/kaito diff --git a/cmd/main.go b/cmd/main.go index 5d18735e9..456442a5b 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -8,6 +8,7 @@ import ( "strconv" "time" + "github.com/azure/kaito/pkg/featuregates" "github.com/azure/kaito/pkg/k8sclient" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/karpenter/pkg/apis/v1beta1" @@ -64,6 +65,7 @@ func main() { var enableLeaderElection bool var enableWebhook bool var probeAddr string + var featureGates string flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, @@ -71,6 +73,7 @@ func main() { "Enabling this will ensure there is only one active controller manager.") flag.BoolVar(&enableWebhook, "webhook", true, "Enable webhook for controller manager. Default is true.") + flag.StringVar(&featureGates, "feature-gates", "Karpenter=false", "Enable Kaito feature gates. Default, Karpenter=false.") opts := zap.Options{ Development: true, } @@ -144,6 +147,11 @@ func main() { // wait 2 seconds to allow reconciling webhookconfiguration and service endpoint. time.Sleep(2 * time.Second) + + if err = featuregates.ParseAndValidateFeatureGates(featureGates); err != nil { + klog.ErrorS(err, "unable to set `feature-gates` flag") + exitWithErrorFunc() + } } klog.InfoS("starting manager") diff --git a/pkg/controllers/workspace_controller.go b/pkg/controllers/workspace_controller.go index 9d41e9c35..4ef8d6871 100644 --- a/pkg/controllers/workspace_controller.go +++ b/pkg/controllers/workspace_controller.go @@ -10,6 +10,7 @@ import ( "time" "github.com/azure/kaito/pkg/tuning" + "github.com/azure/kaito/pkg/utils/consts" batchv1 "k8s.io/api/batch/v1" "github.com/aws/karpenter-core/pkg/apis/v1alpha5" @@ -66,8 +67,8 @@ func (c *WorkspaceReconciler) Reconcile(ctx context.Context, req reconcile.Reque return c.deleteWorkspace(ctx, workspaceObj) } else { // Ensure finalizer - if !controllerutil.ContainsFinalizer(workspaceObj, utils.WorkspaceFinalizer) { - controllerutil.AddFinalizer(workspaceObj, utils.WorkspaceFinalizer) + if !controllerutil.ContainsFinalizer(workspaceObj, consts.WorkspaceFinalizer) { + controllerutil.AddFinalizer(workspaceObj, consts.WorkspaceFinalizer) updateCopy := workspaceObj.DeepCopy() if updateErr := c.Update(ctx, updateCopy, &client.UpdateOptions{}); updateErr != nil { klog.ErrorS(updateErr, "failed to ensure the finalizer to the workspace", diff --git a/pkg/controllers/workspace_gc_finalizer.go b/pkg/controllers/workspace_gc_finalizer.go index ffaae8826..6e985ec0c 100644 --- a/pkg/controllers/workspace_gc_finalizer.go +++ b/pkg/controllers/workspace_gc_finalizer.go @@ -7,7 +7,7 @@ import ( kaitov1alpha1 "github.com/azure/kaito/api/v1alpha1" "github.com/azure/kaito/pkg/machine" - "github.com/azure/kaito/pkg/utils" + "github.com/azure/kaito/pkg/utils/consts" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,6 +39,6 @@ func (c *WorkspaceReconciler) garbageCollectWorkspace(ctx context.Context, wObj } klog.InfoS("successfully removed the workspace finalizers", "workspace", klog.KObj(wObj)) - controllerutil.RemoveFinalizer(wObj, utils.WorkspaceFinalizer) + controllerutil.RemoveFinalizer(wObj, consts.WorkspaceFinalizer) return ctrl.Result{}, nil } diff --git a/pkg/featuregates/featuregates.go b/pkg/featuregates/featuregates.go new file mode 100644 index 000000000..dd9f67957 --- /dev/null +++ b/pkg/featuregates/featuregates.go @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package featuregates + +import ( + "errors" + "fmt" + + "github.com/azure/kaito/pkg/utils/consts" + cliflag "k8s.io/component-base/cli/flag" +) + +var ( + // FeatureGates is a map that holds the feature gates and their default values for Kaito. + FeatureGates = map[string]bool{ + consts.FeatureFlagKarpenter: false, + // Add more feature gates here + } +) + +// ParseAndValidateFeatureGates parses the feature gates flag and sets the environment variables for each feature. +func ParseAndValidateFeatureGates(featureGates string) error { + gateMap := map[string]bool{} + if err := cliflag.NewMapStringBool(&gateMap).Set(featureGates); err != nil { + return err + } + if len(gateMap) == 0 { + // no feature gates set + return nil + } + + var invalidFeatures string + for key, val := range gateMap { + if _, ok := FeatureGates[key]; !ok { + invalidFeatures = fmt.Sprintf("%s, %s", invalidFeatures, key) + continue + } + FeatureGates[key] = val + } + + if invalidFeatures != "" { + return errors.New("invalid feature gate(s) " + invalidFeatures) + } + + return nil +} diff --git a/pkg/featuregates/featuregates_test.go b/pkg/featuregates/featuregates_test.go new file mode 100644 index 000000000..46a3c33db --- /dev/null +++ b/pkg/featuregates/featuregates_test.go @@ -0,0 +1,63 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +package featuregates + +import ( + "testing" + + "gotest.tools/assert" +) + +func TestParseFeatureGates(t *testing.T) { + tests := []struct { + name string + featureGates string + expectedError bool + expectedValue string + }{ + { + name: "WithValidEnableFeatureGates", + featureGates: "Karpenter=true", + expectedError: false, + expectedValue: "true", + }, + { + name: "WithDuplicateFeatureGates", + featureGates: "Karpenter=false,Karpenter=true", + expectedError: false, + expectedValue: "true", // Apply the last value. + }, + { + name: "WithInvalidFeatureGates", + featureGates: "invalid", + expectedError: true, + }, + { + name: "WithUnsupportedFeatureGate", + featureGates: "unsupported=true,Karpenter=false", + expectedError: true, + }, + { + name: "WithValidDisableFeatureGates", + featureGates: "Karpenter=false", + expectedError: false, + expectedValue: "false", + }, + { + name: "WithEmptyFeatureGates", + featureGates: "", + expectedError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ParseAndValidateFeatureGates(tt.featureGates) + if tt.expectedError { + assert.Check(t, err != nil, "expected error but got nil") + } else { + assert.NilError(t, err) + } + }) + } +} diff --git a/pkg/tuning/preset-tuning_test.go b/pkg/tuning/preset-tuning_test.go index cb9fb94e9..14ddaf5d2 100644 --- a/pkg/tuning/preset-tuning_test.go +++ b/pkg/tuning/preset-tuning_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/azure/kaito/pkg/utils" + "github.com/azure/kaito/pkg/utils/consts" kaitov1alpha1 "github.com/azure/kaito/api/v1alpha1" "github.com/azure/kaito/pkg/model" @@ -169,7 +170,7 @@ func TestEnsureTuningConfigMap(t *testing.T) { }{ "Config already exists in workspace namespace": { callMocks: func(c *test.MockClient) { - os.Setenv("RELEASE_NAMESPACE", "release-namespace") + os.Setenv(consts.DefaultReleaseNamespaceEnvVar, "release-namespace") c.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&corev1.ConfigMap{}), mock.Anything).Return(nil) }, workspaceObj: &kaitov1alpha1.Workspace{ @@ -192,7 +193,7 @@ func TestEnsureTuningConfigMap(t *testing.T) { }, "Config doesn't exist in template namespace": { callMocks: func(c *test.MockClient) { - os.Setenv("RELEASE_NAMESPACE", "release-namespace") + os.Setenv(consts.DefaultReleaseNamespaceEnvVar, "release-namespace") c.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&corev1.ConfigMap{}), mock.Anything).Return(errors.NewNotFound(schema.GroupResource{}, "config-template")) }, workspaceObj: &kaitov1alpha1.Workspace{ diff --git a/pkg/utils/common.go b/pkg/utils/common.go index 1f6c584c0..f2bf7a368 100644 --- a/pkg/utils/common.go +++ b/pkg/utils/common.go @@ -1,17 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. + package utils import ( "fmt" "io/ioutil" "os" -) -const ( - // WorkspaceFinalizer is used to make sure that workspace controller handles garbage collection. - WorkspaceFinalizer = "workspace.finalizer.kaito.sh" - DefaultReleaseNamespaceEnvVar = "RELEASE_NAMESPACE" + "github.com/azure/kaito/pkg/utils/consts" ) func Contains(s []string, e string) bool { @@ -76,8 +73,8 @@ func GetReleaseNamespace() (string, error) { } // Fallback: Read the namespace from an environment variable - if namespace, exists := os.LookupEnv(DefaultReleaseNamespaceEnvVar); exists { + if namespace, exists := os.LookupEnv(consts.DefaultReleaseNamespaceEnvVar); exists { return namespace, nil } - return "", fmt.Errorf("failed to determine release namespace from file %s and env var %s", namespaceFilePath, DefaultReleaseNamespaceEnvVar) + return "", fmt.Errorf("failed to determine release namespace from file %s and env var %s", namespaceFilePath, consts.DefaultReleaseNamespaceEnvVar) } diff --git a/pkg/utils/consts/consts.go b/pkg/utils/consts/consts.go new file mode 100644 index 000000000..d6ed62760 --- /dev/null +++ b/pkg/utils/consts/consts.go @@ -0,0 +1,11 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package consts + +const ( + // WorkspaceFinalizer is used to make sure that workspace controller handles garbage collection. + WorkspaceFinalizer = "workspace.finalizer.kaito.sh" + DefaultReleaseNamespaceEnvVar = "RELEASE_NAMESPACE" + FeatureFlagKarpenter = "Karpenter" +)