From 9ff3da42fe0537d55b6d0b9cc4f5e186f324fe95 Mon Sep 17 00:00:00 2001 From: Hardik Dodiya Date: Mon, 11 Nov 2024 09:52:04 +0000 Subject: [PATCH] Allow Processing BootConfigs without IgnitionSecRefs and Remove deprecated MachineBootConfig Reconciler --- .golangci.yml | 1 + api/v1alpha1/httpbootconfig_types.go | 8 +- api/v1alpha1/zz_generated.deepcopy.go | 4 +- cmd/main.go | 24 +-- .../boot.ironcore.dev_httpbootconfigs.yaml | 42 +--- config/rbac/role.yaml | 4 - .../controller/httpbootconfig_controller.go | 4 +- ...achinebootconfiguration_http_controller.go | 183 ------------------ ...serverbootconfiguration_http_controller.go | 11 +- .../serverbootconfiguration_pxe_controller.go | 15 +- server/bootserver.go | 2 +- 11 files changed, 35 insertions(+), 263 deletions(-) delete mode 100644 internal/controller/machinebootconfiguration_http_controller.go diff --git a/.golangci.yml b/.golangci.yml index a32171d..b153cb8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,6 +18,7 @@ issues: - lll - path: "server/*" linters: + - dupl - lll - path: "cmd/*" linters: diff --git a/api/v1alpha1/httpbootconfig_types.go b/api/v1alpha1/httpbootconfig_types.go index 640f274..f839aa6 100644 --- a/api/v1alpha1/httpbootconfig_types.go +++ b/api/v1alpha1/httpbootconfig_types.go @@ -10,10 +10,10 @@ import ( // HTTPBootConfigSpec defines the desired state of HTTPBootConfig type HTTPBootConfigSpec struct { - SystemUUID string `json:"systemUUID,omitempty"` - IgnitionSecretRef *corev1.ObjectReference `json:"ignitionSecretRef,omitempty"` - SystemIPs []string `json:"systemIPs,omitempty"` - UKIURL string `json:"ukiURL,omitempty"` + SystemUUID string `json:"systemUUID,omitempty"` + IgnitionSecretRef *corev1.LocalObjectReference `json:"ignitionSecretRef,omitempty"` + SystemIPs []string `json:"systemIPs,omitempty"` + UKIURL string `json:"ukiURL,omitempty"` } // HTTPBootConfigStatus defines the observed state of HTTPBootConfig diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 99ad984..edf75db 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -8,7 +8,7 @@ package v1alpha1 import ( - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -76,7 +76,7 @@ func (in *HTTPBootConfigSpec) DeepCopyInto(out *HTTPBootConfigSpec) { *out = *in if in.IgnitionSecretRef != nil { in, out := &in.IgnitionSecretRef, &out.IgnitionSecretRef - *out = new(v1.ObjectReference) + *out = new(v1.LocalObjectReference) **out = **in } if in.SystemIPs != nil { diff --git a/cmd/main.go b/cmd/main.go index 90cefcf..16c7574 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -43,11 +43,10 @@ var ( const ( // core controllers - machineBootConfigControllerHttp = "machinebootconfighttp" - ipxeBootConfigController = "ipxebootconfig" - serverBootConfigControllerPxe = "serverbootconfigpxe" - httpBootConfigController = "httpbootconfig" - serverBootConfigControllerHttp = "serverbootconfighttp" + ipxeBootConfigController = "ipxebootconfig" + serverBootConfigControllerPxe = "serverbootconfigpxe" + httpBootConfigController = "httpbootconfig" + serverBootConfigControllerHttp = "serverbootconfighttp" ) func init() { @@ -76,7 +75,6 @@ func main() { var ipxeServiceProtocol string var ipxeServicePort int var imageServerURL string - var bootconfigNamespace string flag.IntVar(&ipxeServicePort, "ipxe-service-port", 5000, "IPXE Service port to listen on.") flag.StringVar(&ipxeServiceProtocol, "ipxe-service-protocol", "http", "IPXE Service Protocol.") @@ -85,7 +83,6 @@ func main() { 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.StringVar(&bootserverAddr, "boot-server-address", ":8082", "The address the boot-server binds to.") - flag.StringVar(&bootconfigNamespace, "machinebootconfig-namespace", "default", "The namespace in which HTTPBootConfigs should be created for MachineBootConfiguration Controller.") flag.StringVar(&imageProxyServerAddr, "image-proxy-server-address", ":8083", "The address the image-proxy-server binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ @@ -100,7 +97,6 @@ func main() { ipxeBootConfigController, serverBootConfigControllerPxe, serverBootConfigControllerHttp, - machineBootConfigControllerHttp, httpBootConfigController, ) @@ -213,18 +209,6 @@ func main() { } } - if controllers.Enabled(machineBootConfigControllerHttp) { - if err = (&controller.MachineBootConfigurationHTTPReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - ImageServerURL: imageServerURL, - BootConfigNamespace: bootconfigNamespace, - }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "MachineBootConfigHttp") - os.Exit(1) - } - } - if controllers.Enabled(httpBootConfigController) { if err = (&controller.HTTPBootConfigReconciler{ Client: mgr.GetClient(), diff --git a/config/crd/bases/boot.ironcore.dev_httpbootconfigs.yaml b/config/crd/bases/boot.ironcore.dev_httpbootconfigs.yaml index 2f3d19e..aa4dd63 100644 --- a/config/crd/bases/boot.ironcore.dev_httpbootconfigs.yaml +++ b/config/crd/bases/boot.ironcore.dev_httpbootconfigs.yaml @@ -47,47 +47,19 @@ spec: description: HTTPBootConfigSpec defines the desired state of HTTPBootConfig properties: ignitionSecretRef: - description: ObjectReference contains enough information to let you - inspect or modify the referred object. + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. properties: - apiVersion: - description: API version of the referent. - type: string - fieldPath: - description: |- - If referring to a piece of an object instead of an entire object, this string - should contain a valid JSON/Go field access statement, such as desiredState.manifest.containers[2]. - For example, if the object reference is to a container within a pod, this would take on a value like: - "spec.containers{name}" (where "name" refers to the name of the container that triggered - the event) or if no container name is specified "spec.containers[2]" (container with - index 2 in this pod). This syntax is chosen only to have some well-defined way of - referencing a part of an object. - type: string - kind: - description: |- - Kind of the referent. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds - type: string name: + default: "" description: |- Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names type: string - namespace: - description: |- - Namespace of the referent. - More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/ - type: string - resourceVersion: - description: |- - Specific resourceVersion to which this reference is made, if any. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency - type: string - uid: - description: |- - UID of the referent. - More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids - type: string type: object x-kubernetes-map-type: atomic systemIPs: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index fda87a8..3b579fa 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -63,8 +63,6 @@ rules: - apiGroups: - metal.ironcore.dev resources: - - bootconfigurations - - machines - serverbootconfigurations - servers verbs: @@ -74,14 +72,12 @@ rules: - apiGroups: - metal.ironcore.dev resources: - - bootconfigurations/finalizers - serverbootconfigurations/finalizers verbs: - update - apiGroups: - metal.ironcore.dev resources: - - bootconfigurations/status - serverbootconfigurations/status verbs: - get diff --git a/internal/controller/httpbootconfig_controller.go b/internal/controller/httpbootconfig_controller.go index ceca43b..a746955 100644 --- a/internal/controller/httpbootconfig_controller.go +++ b/internal/controller/httpbootconfig_controller.go @@ -75,7 +75,7 @@ func (r *HTTPBootConfigReconciler) ensureIgnition(ctx context.Context, _ logr.Lo // Verify if the IgnitionRef is set, and it has the intended data key. if HTTPBootConfig.Spec.IgnitionSecretRef != nil { IgnitionSecret := &corev1.Secret{} - if err := r.Get(ctx, client.ObjectKey{Name: HTTPBootConfig.Spec.IgnitionSecretRef.Name, Namespace: HTTPBootConfig.Spec.IgnitionSecretRef.Namespace}, IgnitionSecret); err != nil { + if err := r.Get(ctx, client.ObjectKey{Name: HTTPBootConfig.Spec.IgnitionSecretRef.Name, Namespace: HTTPBootConfig.Namespace}, IgnitionSecret); err != nil { return bootv1alpha1.HTTPBootConfigStateError, err // TODO: Add some validation steps to ensure that the IgntionData is compliant, if necessary. // Assume for now, that it's going to json format. @@ -143,7 +143,7 @@ func (r *HTTPBootConfigReconciler) enqueueHTTPBootConfigReferencingIgnitionSecre for _, HTTPBootConfig := range list.Items { if HTTPBootConfig.Spec.IgnitionSecretRef != nil && HTTPBootConfig.Spec.IgnitionSecretRef.Name == secretObj.Name && - HTTPBootConfig.Spec.IgnitionSecretRef.Namespace == secretObj.Namespace { + HTTPBootConfig.Namespace == secretObj.Namespace { requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ Name: HTTPBootConfig.Name, diff --git a/internal/controller/machinebootconfiguration_http_controller.go b/internal/controller/machinebootconfiguration_http_controller.go deleted file mode 100644 index 638f912..0000000 --- a/internal/controller/machinebootconfiguration_http_controller.go +++ /dev/null @@ -1,183 +0,0 @@ -// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors -// SPDX-License-Identifier: Apache-2.0 - -package controller - -import ( - "context" - "fmt" - "strings" - - "github.com/go-logr/logr" - bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/predicate" - - metalv1alpha1 "github.com/ironcore-dev/metal/api/v1alpha1" -) - -type MachineBootConfigurationHTTPReconciler struct { - client.Client - Scheme *runtime.Scheme - ImageServerURL string - BootConfigNamespace string -} - -//+kubebuilder:rbac:groups=metal.ironcore.dev,resources=bootconfigurations,verbs=get;list;watch -//+kubebuilder:rbac:groups=metal.ironcore.dev,resources=bootconfigurations/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=metal.ironcore.dev,resources=bootconfigurations/finalizers,verbs=update -//+kubebuilder:rbac:groups=boot.ironcore.dev,resources=httpbootconfig,verbs=get;list;watch;create;delete;patch -//+kubebuilder:rbac:groups=boot.ironcore.dev,resources=httpbootconfig/status,verbs=get -//+kubebuilder:rbac:groups=metal.ironcore.dev,resources=machines,verbs=get;list;watch - -func (r *MachineBootConfigurationHTTPReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - log := ctrl.LoggerFrom(ctx) - bootConfig := &metalv1alpha1.BootConfiguration{} - if err := r.Get(ctx, req.NamespacedName, bootConfig); err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) - } - - return r.reconcileExists(ctx, log, bootConfig) -} - -func (r *MachineBootConfigurationHTTPReconciler) reconcileExists(ctx context.Context, log logr.Logger, config *metalv1alpha1.BootConfiguration) (ctrl.Result, error) { - if !config.DeletionTimestamp.IsZero() { - return r.delete(ctx, log, config) - } - return r.reconcile(ctx, log, config) -} - -func (r *MachineBootConfigurationHTTPReconciler) delete(ctx context.Context, log logr.Logger, config *metalv1alpha1.BootConfiguration) (ctrl.Result, error) { - // TODO - return ctrl.Result{}, nil -} - -func (r *MachineBootConfigurationHTTPReconciler) reconcile(ctx context.Context, log logr.Logger, config *metalv1alpha1.BootConfiguration) (ctrl.Result, error) { - log.V(1).Info("Reconciling BootConfiguration for HTTPBoot") - - systemUUID, err := r.getSystemUUIDFromMachine(ctx, config) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to get system UUID from Machine: %w", err) - } - log.V(1).Info("Got system UUID from Machine", "systemUUID", systemUUID) - - systemIPs, err := r.getSystemIPFromMachine(ctx, config) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to get system IPs from Machine: %w", err) - } - log.V(1).Info("Got system IPs from Machine", "systemIPs", systemIPs) - - ukiURL := r.constructUKIURL(config.Spec.Image) - log.V(1).Info("Extracted UKI URL for boot") - - httpBootConfig := &bootv1alpha1.HTTPBootConfig{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "boot.ironcore.dev/v1alpha1", - Kind: "HTTPBootConfig", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: r.BootConfigNamespace, - Name: config.Name, - }, - Spec: bootv1alpha1.HTTPBootConfigSpec{ - SystemUUID: systemUUID, - SystemIPs: systemIPs, - UKIURL: ukiURL, - IgnitionSecretRef: &corev1.ObjectReference{Name: config.Spec.IgnitionSecretRef.Name, Namespace: config.Spec.IgnitionSecretRef.Namespace}, - }, - } - - if err := controllerutil.SetControllerReference(config, httpBootConfig, r.Scheme); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to set controller reference: %w", err) - } - log.V(1).Info("Set controller reference") - - if err := r.Patch(ctx, httpBootConfig, client.Apply, client.FieldOwner("machine-boot-controller"), client.ForceOwnership); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to apply HTTPBoot config: %w", err) - } - log.V(1).Info("Applied HTTPBoot config for machine boot configuration") - - if err := r.Get(ctx, client.ObjectKey{Namespace: r.BootConfigNamespace, Name: config.Name}, httpBootConfig); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to get HTTPBoot config: %w", err) - } - - if err := r.patchConfigStateFromHTTPState(ctx, httpBootConfig, config); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch machine boot config state to %s: %w", httpBootConfig.Status.State, err) - } - log.V(1).Info("Patched machine boot config state") - - log.V(1).Info("Reconciled BootConfiguration") - - return ctrl.Result{}, nil -} - -func (r *MachineBootConfigurationHTTPReconciler) patchConfigStateFromHTTPState(ctx context.Context, httpBootConfig *bootv1alpha1.HTTPBootConfig, config *metalv1alpha1.BootConfiguration) error { - if httpBootConfig.Status.State == bootv1alpha1.HTTPBootConfigStateReady { - return r.patchState(ctx, config, metalv1alpha1.BootConfigurationStateReady) - } - - if httpBootConfig.Status.State == bootv1alpha1.HTTPBootConfigStateError { - return r.patchState(ctx, config, metalv1alpha1.BootConfigurationStateError) - } - return nil -} - -func (r *MachineBootConfigurationHTTPReconciler) patchState(ctx context.Context, config *metalv1alpha1.BootConfiguration, state metalv1alpha1.BootConfigurationState) error { - configBase := config.DeepCopy() - config.Status.State = state - if err := r.Status().Patch(ctx, config, client.MergeFrom(configBase)); err != nil { - return err - } - return nil -} - -// getSystemUUIDFromMachine fetches the UUID from the referenced Machine object. -func (r *MachineBootConfigurationHTTPReconciler) getSystemUUIDFromMachine(ctx context.Context, config *metalv1alpha1.BootConfiguration) (string, error) { - machine := &metalv1alpha1.Machine{} - if err := r.Get(ctx, client.ObjectKey{Name: config.Spec.MachineRef.Name}, machine); err != nil { - return "", fmt.Errorf("failed to get Machine: %w", err) - } - return machine.Spec.UUID, nil -} - -// getSystemIPFromMachine fetches the IPs from the network interfaces of the referenced Machine object. -func (r *MachineBootConfigurationHTTPReconciler) getSystemIPFromMachine(ctx context.Context, config *metalv1alpha1.BootConfiguration) ([]string, error) { - machine := &metalv1alpha1.Machine{} - if err := r.Get(ctx, client.ObjectKey{Name: config.Spec.MachineRef.Name}, machine); err != nil { - return nil, fmt.Errorf("failed to get Machine: %w", err) - } - - systemIPs := make([]string, 0, 2*len(machine.Status.NetworkInterfaces)) - - for _, nic := range machine.Status.NetworkInterfaces { - systemIPs = append(systemIPs, nic.IPRef.Name) - systemIPs = append(systemIPs, nic.MacAddress) - } - return systemIPs, nil -} - -func (r *MachineBootConfigurationHTTPReconciler) constructUKIURL(image string) string { - sanitizedImage := strings.Replace(image, "/", "-", -1) - sanitizedImage = strings.Replace(sanitizedImage, ":", "-", -1) - sanitizedImage = strings.Replace(sanitizedImage, "https://", "", -1) - sanitizedImage = strings.Replace(sanitizedImage, "http://", "", -1) - - filename := fmt.Sprintf("%s-uki.efi", sanitizedImage) - - ukiURL := fmt.Sprintf("%s/%s", r.ImageServerURL, filename) - return ukiURL -} - -// SetupWithManager sets up the controller with the Manager. -func (r *MachineBootConfigurationHTTPReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - For(&metalv1alpha1.BootConfiguration{}). - Owns(&bootv1alpha1.HTTPBootConfig{}). - WithEventFilter(predicate.ResourceVersionChangedPredicate{}). - Complete(r) -} diff --git a/internal/controller/serverbootconfiguration_http_controller.go b/internal/controller/serverbootconfiguration_http_controller.go index 7335aa8..fb8331f 100644 --- a/internal/controller/serverbootconfiguration_http_controller.go +++ b/internal/controller/serverbootconfiguration_http_controller.go @@ -10,7 +10,6 @@ import ( "github.com/go-logr/logr" bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -83,12 +82,14 @@ func (r *ServerBootConfigurationHTTPReconciler) reconcile(ctx context.Context, l Name: config.Name, }, Spec: bootv1alpha1.HTTPBootConfigSpec{ - SystemUUID: systemUUID, - SystemIPs: systemIPs, - UKIURL: ukiURL, - IgnitionSecretRef: &corev1.ObjectReference{Name: config.Spec.IgnitionSecretRef.Name, Namespace: config.Namespace}, + SystemUUID: systemUUID, + SystemIPs: systemIPs, + UKIURL: ukiURL, }, } + if config.Spec.IgnitionSecretRef != nil { + httpBootConfig.Spec.IgnitionSecretRef = config.Spec.IgnitionSecretRef + } if err := controllerutil.SetControllerReference(config, httpBootConfig, r.Scheme); err != nil { return ctrl.Result{}, fmt.Errorf("failed to set controller reference: %w", err) diff --git a/internal/controller/serverbootconfiguration_pxe_controller.go b/internal/controller/serverbootconfiguration_pxe_controller.go index 6141a88..4de2e2c 100644 --- a/internal/controller/serverbootconfiguration_pxe_controller.go +++ b/internal/controller/serverbootconfiguration_pxe_controller.go @@ -23,7 +23,6 @@ import ( "github.com/ironcore-dev/boot-operator/api/v1alpha1" ironcoreimage "github.com/ironcore-dev/ironcore-image" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -101,14 +100,16 @@ func (r *ServerBootConfigurationPXEReconciler) reconcile(ctx context.Context, lo Name: config.Name, }, Spec: v1alpha1.IPXEBootConfigSpec{ - SystemUUID: systemUUID, - SystemIPs: systemIPs, - KernelURL: kernelURL, - InitrdURL: initrdURL, - SquashfsURL: squashFSURL, - IgnitionSecretRef: &v1.LocalObjectReference{Name: config.Spec.IgnitionSecretRef.Name}, + SystemUUID: systemUUID, + SystemIPs: systemIPs, + KernelURL: kernelURL, + InitrdURL: initrdURL, + SquashfsURL: squashFSURL, }, } + if config.Spec.IgnitionSecretRef != nil { + ipxeConfig.Spec.IgnitionSecretRef = config.Spec.IgnitionSecretRef + } if err := controllerutil.SetControllerReference(config, ipxeConfig, r.Scheme); err != nil { return ctrl.Result{}, fmt.Errorf("failed to set controller reference: %w", err) diff --git a/server/bootserver.go b/server/bootserver.go index 7cd91d9..8529223 100644 --- a/server/bootserver.go +++ b/server/bootserver.go @@ -277,7 +277,7 @@ func handleIgnitionHTTPBoot(w http.ResponseWriter, r *http.Request, k8sClient cl ignitionSecret := corev1.Secret{ ObjectMeta: v1.ObjectMeta{ Name: httpBootConfig.Spec.IgnitionSecretRef.Name, - Namespace: httpBootConfig.Spec.IgnitionSecretRef.Namespace, + Namespace: httpBootConfig.Namespace, }, } ignitionData, ignitionFormat, err := fetchIgnitionData(ctx, k8sClient, ignitionSecret)