From 86b1cc00ec6b0225794e6633dd688af496c89648 Mon Sep 17 00:00:00 2001 From: Antonin Stefanutti Date: Thu, 1 Jun 2023 15:53:38 +0200 Subject: [PATCH] feat(api): Support custom InstaScale container image --- Makefile | 38 ++++++++++++++++--- api/v1alpha1/instascale_types.go | 17 +++++++-- .../codeflare.codeflare.dev_instascales.yaml | 11 ++++++ .../internal/instascale/deployment.yaml.tmpl | 2 +- controllers/defaults.go | 9 +++++ controllers/instascale_controller.go | 38 +++++++++---------- controllers/instascale_params.go | 18 ++++++--- 7 files changed, 99 insertions(+), 34 deletions(-) create mode 100644 controllers/defaults.go diff --git a/Makefile b/Makefile index 182059677..343760b10 100644 --- a/Makefile +++ b/Makefile @@ -5,6 +5,9 @@ # - use environment variables to overwrite this value (e.g export VERSION=0.0.2) VERSION ?= 0.0.2 +# INSTASCALE_VERSION defines the default version of the InstaScale controller +INSTASCALE_VERSION ?= v0.0.3 + # CHANNELS define the bundle channels used in the bundle. # Add a new line here if you would like to change its default config. (E.g CHANNELS = "candidate,fast,stable") # To re-generate a bundle for other specific channels without changing the standard setup, you can: @@ -24,12 +27,18 @@ BUNDLE_DEFAULT_CHANNEL := --default-channel=$(DEFAULT_CHANNEL) endif BUNDLE_METADATA_OPTS ?= $(BUNDLE_CHANNELS) $(BUNDLE_DEFAULT_CHANNEL) +# IMAGE_ORG_BASE defines the base container registry and organization for container images. +IMAGE_ORG_BASE ?= quay.io/project-codeflare + # IMAGE_TAG_BASE defines the docker.io namespace and part of the image name for remote images. # This variable is used to construct full image tags for bundle and catalog images. # # For example, running 'make bundle-build bundle-push catalog-build catalog-push' will build and push both # codeflare.dev/codeflare-operator-bundle:$VERSION and codeflare.dev/codeflare-operator-catalog:$VERSION. -IMAGE_TAG_BASE ?= quay.io/project-codeflare/codeflare-operator +IMAGE_TAG_BASE ?= $(IMAGE_ORG_BASE)/codeflare-operator + +# INSTASCALE_IMAGE defines the default container image for the InstaScale controller +INSTASCALE_IMAGE ?= $(IMAGE_ORG_BASE)/instascale-controller:$(INSTASCALE_VERSION) # BUNDLE_IMG defines the image:tag used for the bundle. # You can use it as an arg. (E.g make bundle-build BUNDLE_IMG=/:) @@ -85,6 +94,25 @@ help: ## Display this help. ##@ Development +DEFAULTS_FILE := controllers/defaults.go + +.PHONY: defaults +defaults: + $(info Regenerating $(DEFAULTS_FILE)) + @echo "package controllers" > $(DEFAULTS_FILE) + @echo "" >> $(DEFAULTS_FILE) + @echo "// ***********************" >> $(DEFAULTS_FILE) + @echo "// DO NOT EDIT THIS FILE" >> $(DEFAULTS_FILE) + @echo "// ***********************" >> $(DEFAULTS_FILE) + @echo "" >> $(DEFAULTS_FILE) + @echo "const (" >> $(DEFAULTS_FILE) + @echo " InstaScaleImage = \"$(INSTASCALE_IMAGE)\"" >> $(DEFAULTS_FILE) + @echo "" >> $(DEFAULTS_FILE) + @echo ")" >> $(DEFAULTS_FILE) + @echo "" >> $(DEFAULTS_FILE) + + gofmt -w $(DEFAULTS_FILE) + .PHONY: manifests manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. $(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases @@ -105,11 +133,11 @@ vet: ## Run go vet against code. ##@ Build .PHONY: build -build: generate fmt vet ## Build manager binary. +build: defaults generate fmt vet ## Build manager binary. go build -o bin/manager main.go .PHONY: run -run: manifests generate fmt vet ## Run a controller from your host. +run: defaults manifests generate fmt vet ## Run a controller from your host. go run ./main.go .PHONY: image-build @@ -189,7 +217,7 @@ validate-bundle: install-operator-sdk $(OPERATOR_SDK) bundle validate ./bundle --select-optional suite=operatorframework .PHONY: bundle -bundle: manifests kustomize install-operator-sdk ## Generate bundle manifests and metadata, then validate generated files. +bundle: defaults manifests kustomize install-operator-sdk ## Generate bundle manifests and metadata, then validate generated files. $(OPERATOR_SDK) generate kustomize manifests -q cd config/manager && $(KUSTOMIZE) edit set image controller=$(IMG) cd config/manifests && $(KUSTOMIZE) edit add patch --patch '[{"op":"add", "path":"/metadata/annotations/containerImage", "value": "$(IMG)" }]' --kind ClusterServiceVersion @@ -246,5 +274,5 @@ catalog-push: ## Push a catalog image. $(MAKE) image-push IMG=$(CATALOG_IMG) .PHONY: test-unit -test-unit: manifests generate fmt vet envtest ## Run tests. +test-unit: defaults manifests generate fmt vet envtest ## Run tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -coverprofile cover.out diff --git a/api/v1alpha1/instascale_types.go b/api/v1alpha1/instascale_types.go index b361654b6..063f9a0d4 100644 --- a/api/v1alpha1/instascale_types.go +++ b/api/v1alpha1/instascale_types.go @@ -38,6 +38,17 @@ type InstaScaleSpec struct { // controllerResources determines the container resources for the InstaScale controller deployment ControllerResources *v1.ResourceRequirements `json:"controllerResources,omitempty"` + + // The container image for the InstaScale controller deployment. + // If specified, the provided container image must be compatible with the running CodeFlare operator. + // Using an incompatible, or unrelated container image, will result in an undefined behavior. + // A CodeFlare operator upgrade will not upgrade the InstaScale controller, that'll keep running this + // specified container image. + // If not specified, the latest version compatible with the running CodeFlare operator is used. + // A CodeFlare operator upgrade may upgrade the InstaScale controller to a newer container image. + // + // +optional + ControllerImage string `json:"controllerImage,omitempty"` } // InstaScaleStatus defines the observed state of InstaScale @@ -48,8 +59,8 @@ type InstaScaleStatus struct { Ready bool `json:"ready"` } -//+kubebuilder:object:root=true -//+kubebuilder:subresource:status +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status // InstaScale is the Schema for the instascales API // +operator-sdk:csv:customresourcedefinitions:displayName="InstaScale" @@ -61,7 +72,7 @@ type InstaScale struct { Status InstaScaleStatus `json:"status,omitempty"` } -//+kubebuilder:object:root=true +// +kubebuilder:object:root=true // InstaScaleList contains a list of InstaScale type InstaScaleList struct { diff --git a/config/crd/bases/codeflare.codeflare.dev_instascales.yaml b/config/crd/bases/codeflare.codeflare.dev_instascales.yaml index 07a21ec74..26a72b760 100644 --- a/config/crd/bases/codeflare.codeflare.dev_instascales.yaml +++ b/config/crd/bases/codeflare.codeflare.dev_instascales.yaml @@ -35,6 +35,17 @@ spec: spec: description: InstaScaleSpec defines the desired state of InstaScale properties: + controllerImage: + description: The container image for the InstaScale controller deployment. + If specified, the provided container image must be compatible with + the running CodeFlare operator. Using an incompatible, or unrelated + container image, will result in an undefined behavior. A CodeFlare + operator upgrade will not upgrade the InstaScale controller, that'll + keep running this specified container image. If not specified, the + latest version compatible with the running CodeFlare operator is + used. A CodeFlare operator upgrade may upgrade the InstaScale controller + to a newer container image. + type: string controllerResources: description: controllerResources determines the container resources for the InstaScale controller deployment diff --git a/config/internal/instascale/deployment.yaml.tmpl b/config/internal/instascale/deployment.yaml.tmpl index 13df3a0da..4c704c80c 100644 --- a/config/internal/instascale/deployment.yaml.tmpl +++ b/config/internal/instascale/deployment.yaml.tmpl @@ -18,7 +18,7 @@ spec: containers: - args: - "--configs-namespace={{.Namespace}}" - image: quay.io/project-codeflare/instascale-controller:v0.0.3 + image: {{.ControllerImage}} name: instascale resources: {{.ControllerResources}} serviceAccountName: instascale-{{.Name}}-sa diff --git a/controllers/defaults.go b/controllers/defaults.go new file mode 100644 index 000000000..a0524c7c3 --- /dev/null +++ b/controllers/defaults.go @@ -0,0 +1,9 @@ +package controllers + +// *********************** +// DO NOT EDIT THIS FILE +// *********************** + +const ( + InstaScaleImage = "quay.io/project-codeflare/instascale-controller:v0.0.3" +) diff --git a/controllers/instascale_controller.go b/controllers/instascale_controller.go index 4b52bd123..c9fabdf57 100644 --- a/controllers/instascale_controller.go +++ b/controllers/instascale_controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "path" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -27,6 +28,7 @@ import ( apierrs "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -36,6 +38,7 @@ import ( "github.com/go-logr/logr" mf "github.com/manifestival/manifestival" + codeflarev1alpha1 "github.com/project-codeflare/codeflare-operator/api/v1alpha1" "github.com/project-codeflare/codeflare-operator/controllers/config" "github.com/project-codeflare/codeflare-operator/controllers/util" @@ -55,8 +58,7 @@ var instascaleClusterScopedTemplates = []string{ } func (r *InstaScaleReconciler) Apply(owner mf.Owner, params *InstaScaleParams, template string, fns ...mf.Transformer) error { - - tmplManifest, err := config.Manifest(r.Client, r.TemplatesPath+template, params, template, r.Log) + tmplManifest, err := config.Manifest(r.Client, path.Join(r.TemplatesPath, template), params, template, r.Log) if err != nil { return fmt.Errorf("error loading template yaml: %w", err) } @@ -77,17 +79,17 @@ func (r *InstaScaleReconciler) Apply(owner mf.Owner, params *InstaScaleParams, t // TODO: Review node permissions, instascale should only require read -//+kubebuilder:rbac:groups=codeflare.codeflare.dev,resources=instascales,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=codeflare.codeflare.dev,resources=instascales/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=codeflare.codeflare.dev,resources=instascales/finalizers,verbs=update -//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=*,resources=deployments;services,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=core,resources=secrets;configmaps;nodes;services;serviceaccounts;persistentvolumes;persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=core,resources=persistentvolumes;persistentvolumeclaims,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=get;list;watch;create;update;delete -//+kubebuilder:rbac:groups=machine.openshift.io,resources=*,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=mcad.ibm.com,resources=appwrappers;queuejobs;schedulingspecs,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=codeflare.codeflare.dev,resources=instascales,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=codeflare.codeflare.dev,resources=instascales/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=codeflare.codeflare.dev,resources=instascales/finalizers,verbs=update +// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=*,resources=deployments;services,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=core,resources=secrets;configmaps;nodes;services;serviceaccounts;persistentvolumes;persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=core,resources=persistentvolumes;persistentvolumeclaims,verbs=* +// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=get;list;watch;create;update;delete +// +kubebuilder:rbac:groups=machine.openshift.io,resources=*,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=mcad.ibm.com,resources=appwrappers;queuejobs;schedulingspecs,verbs=get;list;watch;create;update;patch;delete // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -124,11 +126,7 @@ func (r *InstaScaleReconciler) Reconcile(ctx context.Context, req ctrl.Request) instascaleCustomResource.APIVersion, instascaleCustomResource.Kind = gvk.Version, gvk.Kind } - err = params.ExtractParams(instascaleCustomResource) - if err != nil { - log.Error(err, "Unable to parse InstaScale custom resource") - return ctrl.Result{}, err - } + params.ExtractParams(instascaleCustomResource) if instascaleCustomResource.ObjectMeta.DeletionTimestamp.IsZero() { if !controllerutil.ContainsFinalizer(instascaleCustomResource, finalizerName) { @@ -152,7 +150,6 @@ func (r *InstaScaleReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } - log.V(1).Info("ReconcileInstaScale called.") err = r.ReconcileInstaScale(instascaleCustomResource, params) if err != nil { return ctrl.Result{}, err @@ -162,7 +159,8 @@ func (r *InstaScaleReconciler) Reconcile(ctx context.Context, req ctrl.Request) if err != nil { return ctrl.Result{}, err } - err = r.Client.Status().Update(context.Background(), instascaleCustomResource) + + err = r.Client.Status().Update(ctx, instascaleCustomResource) if err != nil { return ctrl.Result{}, err } diff --git a/controllers/instascale_params.go b/controllers/instascale_params.go index 1febfb39f..eb6737c79 100644 --- a/controllers/instascale_params.go +++ b/controllers/instascale_params.go @@ -3,21 +3,25 @@ package controllers import ( "encoding/json" - mf "github.com/manifestival/manifestival" - instascalev1alpha1 "github.com/project-codeflare/codeflare-operator/api/v1alpha1" + "github.com/manifestival/manifestival" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + + instascalev1alpha1 "github.com/project-codeflare/codeflare-operator/api/v1alpha1" ) type InstaScaleParams struct { Name string Namespace string - Owner mf.Owner + Owner manifestival.Owner EnableMonitoring bool MaxScaleoutAllowed int UseMachinePools bool ControllerResources ControllerResources + ControllerImage string } + type ControllerResources struct { v1.ResourceRequirements } @@ -29,9 +33,14 @@ func (c *ControllerResources) String() string { } return string(raw) } -func (p *InstaScaleParams) ExtractParams(instascale *instascalev1alpha1.InstaScale) error { + +func (p *InstaScaleParams) ExtractParams(instascale *instascalev1alpha1.InstaScale) { p.Name = instascale.Name p.Namespace = instascale.Namespace + p.ControllerImage = instascale.Spec.ControllerImage + if p.ControllerImage == "" { + p.ControllerImage = InstaScaleImage + } p.Owner = instascale p.EnableMonitoring = instascale.Spec.EnableMonitoring p.MaxScaleoutAllowed = instascale.Spec.MaxScaleoutAllowed @@ -49,5 +58,4 @@ func (p *InstaScaleParams) ExtractParams(instascale *instascalev1alpha1.InstaSca } else { p.ControllerResources = ControllerResources{*instascale.Spec.ControllerResources} } - return nil }