diff --git a/internal/controller/funcs.go b/internal/controller/funcs.go index b7c24175..4b83faec 100644 --- a/internal/controller/funcs.go +++ b/internal/controller/funcs.go @@ -21,22 +21,25 @@ import ( "context" "errors" "fmt" + "strings" "github.com/go-logr/logr" rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1" topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1" keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" + corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac" "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/util" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // Static errors for ironic controllers @@ -96,6 +99,32 @@ func getCommonRbacRules() []rbacv1.PolicyRule { Resources: []string{"securitycontextconstraints"}, Verbs: []string{"use"}, }, + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{"create", "get", "list", "watch", "update", "patch", "delete"}, + }, + } +} + +func getGraphicalConsoleRbacRules() []rbacv1.PolicyRule { + return []rbacv1.PolicyRule{ + { + APIGroups: []string{"security.openshift.io"}, + ResourceNames: []string{"anyuid", "privileged"}, + Resources: []string{"securitycontextconstraints"}, + Verbs: []string{"use"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{"create", "get", "list", "watch", "update", "patch", "delete"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"create", "get", "list", "watch", "update", "patch", "delete"}, + }, } } @@ -166,6 +195,157 @@ func getQuorumQueues( return quorumQueues, nil } +// getConsoleNamespaceName returns the namespace name for console pods based on the service namespace. +// The prefix is extracted from the service namespace (e.g., "openstack" -> "openstack-ironic-consoles") +func getConsoleNamespaceName(serviceNamespace string) string { + // Extract the prefix from the service namespace (before any hyphen or use full name) + prefix := serviceNamespace + if idx := strings.Index(serviceNamespace, "-"); idx > 0 { + prefix = serviceNamespace[:idx] + } + return prefix + "-ironic-consoles" +} + +// ensureConsoleNamespace creates the console namespace if it doesn't exist +func ensureConsoleNamespace( + ctx context.Context, + h *helper.Helper, + serviceNamespace string, +) error { + consolesNamespace := getConsoleNamespaceName(serviceNamespace) + + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: consolesNamespace, + }, + } + + op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), ns, func() error { + // Set labels + if ns.Labels == nil { + ns.Labels = make(map[string]string) + } + ns.Labels["app"] = "ironic" + return nil + }) + + if err != nil { + return fmt.Errorf("failed to reconcile console namespace %s: %w", consolesNamespace, err) + } + + if op != controllerutil.OperationResultNone { + h.GetLogger().Info(fmt.Sprintf("Namespace %s %s", consolesNamespace, op)) + } + + return nil +} + +// reconcileGraphicalConsoleRbac creates a Role and RoleBinding in the console namespace +// that grants the ServiceAccount from the service namespace permissions to create console pods. +// This enables cross-namespace RBAC where the ironic ServiceAccount in the 'openstack' namespace +// can create pods and secrets in the 'openstack-ironic-consoles' namespace. +// Note: These resources cannot have owner references since cross-namespace ownership is not allowed. +func reconcileGraphicalConsoleRbac( + ctx context.Context, + h *helper.Helper, + instance common_rbac.Reconciler, + serviceAccountName string, + consoleNamespace string, + rules []rbacv1.PolicyRule, +) (ctrl.Result, error) { + serviceNamespace := instance.RbacNamespace() + roleName := serviceAccountName + "-console-role" + roleBindingName := serviceAccountName + "-console-rolebinding" + + labels := map[string]string{ + "app": "ironic", + "ironic.openstack.org/service": serviceAccountName, + } + + // Create or update Role in the console namespace without owner references + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: roleName, + Namespace: consoleNamespace, + }, + } + + op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), role, func() error { + // Set labels + role.Labels = labels + // Set rules + role.Rules = rules + return nil + }) + + if err != nil { + instance.RbacConditionsSet(condition.FalseCondition( + condition.RoleReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.RoleReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err + } + + if op != controllerutil.OperationResultNone { + h.GetLogger().Info(fmt.Sprintf("Role %s %s in namespace %s", roleName, op, consoleNamespace)) + } + + instance.RbacConditionsSet(condition.TrueCondition( + condition.RoleReadyCondition, + condition.RoleReadyMessage)) + + // Create or update RoleBinding in the console namespace that references the ServiceAccount + // from the service namespace (cross-namespace reference) + roleBinding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: roleBindingName, + Namespace: consoleNamespace, + }, + } + + op, err = controllerutil.CreateOrPatch(ctx, h.GetClient(), roleBinding, func() error { + // Set labels + roleBinding.Labels = labels + // Set RoleRef (immutable, but safe to set on create) + roleBinding.RoleRef = rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: roleName, + } + // Set Subjects + roleBinding.Subjects = []rbacv1.Subject{ + { + Kind: "ServiceAccount", + Name: serviceAccountName, + Namespace: serviceNamespace, + }, + } + return nil + }) + + if err != nil { + instance.RbacConditionsSet(condition.FalseCondition( + condition.RoleBindingReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.RoleBindingReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err + } + + if op != controllerutil.OperationResultNone { + h.GetLogger().Info(fmt.Sprintf("RoleBinding %s %s in namespace %s", roleBindingName, op, consoleNamespace)) + } + + instance.RbacConditionsSet(condition.TrueCondition( + condition.RoleBindingReadyCondition, + condition.RoleBindingReadyMessage)) + + return ctrl.Result{}, nil +} + // setApplicationCredentialParams - shared function to set ApplicationCredential template parameters // secretName is the name of the secret containing the application credentials // Returns true if application credentials are available and configured diff --git a/internal/controller/ironic_controller.go b/internal/controller/ironic_controller.go index f4b1edb9..5fd18cc6 100644 --- a/internal/controller/ironic_controller.go +++ b/internal/controller/ironic_controller.go @@ -1053,21 +1053,29 @@ func (r *IronicReconciler) conductorDeploymentCreateOrUpdate( IronicConductorSpec := ironicv1.IronicConductorSpec{ IronicConductorTemplate: conductorSpec, - ContainerImage: instance.Spec.Images.Conductor, - PxeContainerImage: instance.Spec.Images.Pxe, - IronicPythonAgentImage: instance.Spec.Images.IronicPythonAgent, - Standalone: instance.Spec.Standalone, - RPCTransport: instance.Spec.RPCTransport, - Secret: instance.Spec.Secret, - PasswordSelectors: instance.Spec.PasswordSelectors, - ServiceUser: instance.Spec.ServiceUser, - DatabaseAccount: instance.Spec.DatabaseAccount, - DatabaseHostname: instance.Status.DatabaseHostname, - TransportURLSecret: transportURLSecretName, - KeystoneEndpoints: *keystoneEndpoints, - Region: keystoneRegion, - TLS: instance.Spec.IronicAPI.TLS.Ca, - Auth: instance.Spec.Auth, + // FIXME(stevebaker) drop this when https://github.com/openstack-k8s-operators/s2i-openstack-containers/pull/259 lands + // ContainerImage: instance.Spec.Images.Conductor, + ContainerImage: "quay.io/steveb/openstack-ironic-conductor:kubectl", + PxeContainerImage: instance.Spec.Images.Pxe, + IronicPythonAgentImage: instance.Spec.Images.IronicPythonAgent, + Standalone: instance.Spec.Standalone, + RPCTransport: instance.Spec.RPCTransport, + Secret: instance.Spec.Secret, + PasswordSelectors: instance.Spec.PasswordSelectors, + ServiceUser: instance.Spec.ServiceUser, + DatabaseAccount: instance.Spec.DatabaseAccount, + DatabaseHostname: instance.Status.DatabaseHostname, + TransportURLSecret: transportURLSecretName, + KeystoneEndpoints: *keystoneEndpoints, + Region: keystoneRegion, + TLS: instance.Spec.IronicAPI.TLS.Ca, + Auth: instance.Spec.Auth, + GraphicalConsoles: instance.Spec.GraphicalConsoles, + // FIXME(stevebaker) drop this when https://github.com/openstack-k8s-operators/openstack-operator/pull/1633 lands + // ConsoleImage: instance.Spec.Images.GraphicalConsole, + // NoVNCProxyImage: instance.Spec.Images.NoVNCProxy, + ConsoleImage: "quay.io/openstack-s2i-containers/openstack-ironic-graphical-console:master-latest", + NoVNCProxyImage: "quay.io/openstack-s2i-containers/openstack-ironic-api:master-latest", } if instance.Status.NotificationsURLSecret != nil { diff --git a/internal/controller/ironicconductor_controller.go b/internal/controller/ironicconductor_controller.go index 324f2bae..edc710a7 100644 --- a/internal/controller/ironicconductor_controller.go +++ b/internal/controller/ironicconductor_controller.go @@ -33,6 +33,7 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" @@ -405,32 +406,25 @@ func (r *IronicConductorReconciler) reconcileServices( if instance.Spec.ConductorGroup != "" { conductorServiceLabels[ironic.ConductorGroupSelector] = strings.ToLower(instance.Spec.ConductorGroup) } - conductorService := ironicconductor.Service(conductorPod.Name, instance, conductorServiceLabels) - if conductorService != nil { - err = controllerutil.SetOwnerReference(&conductorPod, conductorService, helper.GetScheme()) + desiredService := ironicconductor.Service(conductorPod.Name, instance, conductorServiceLabels) + if desiredService != nil { + conductorService := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: desiredService.Name, + Namespace: desiredService.Namespace, + }, + } + op, err := controllerutil.CreateOrPatch(ctx, r.Client, conductorService, func() error { + conductorService.Labels = desiredService.Labels + conductorService.Spec.Selector = desiredService.Spec.Selector + conductorService.Spec.Ports = desiredService.Spec.Ports + return controllerutil.SetOwnerReference(&conductorPod, conductorService, helper.GetScheme()) + }) if err != nil { return ctrl.Result{}, err } - err = r.Get( - ctx, - types.NamespacedName{ - Name: conductorService.Name, - Namespace: conductorService.Namespace, - }, - conductorService, - ) - if err != nil && k8s_errors.IsNotFound(err) { - Log.Info(fmt.Sprintf("Service port %s does not exist, creating it", conductorService.Name)) - err = r.Create(ctx, conductorService) - if err != nil { - return ctrl.Result{}, err - } - } else { - Log.Info(fmt.Sprintf("Service port %s exists, updating it", conductorService.Name)) - err = r.Update(ctx, conductorService) - if err != nil { - return ctrl.Result{}, err - } + if op != controllerutil.OperationResultNone { + Log.Info(fmt.Sprintf("Service %s %s", conductorService.Name, op)) } } // create service - end @@ -449,32 +443,59 @@ func (r *IronicConductorReconciler) reconcileServices( conductorRouteLabels[ironic.ConductorGroupSelector] = strings.ToLower(instance.Spec.ConductorGroup) } - conductorRoute := ironicconductor.Route(conductorPod.Name, instance, conductorRouteLabels) - err = controllerutil.SetOwnerReference(&conductorPod, conductorRoute, helper.GetScheme()) + desiredRoute := ironicconductor.Route(conductorPod.Name, instance, conductorRouteLabels) + conductorRoute := &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: desiredRoute.Name, + Namespace: desiredRoute.Namespace, + }, + } + op, err := controllerutil.CreateOrPatch(ctx, r.Client, conductorRoute, func() error { + conductorRoute.Labels = desiredRoute.Labels + conductorRoute.Spec = desiredRoute.Spec + return controllerutil.SetOwnerReference(&conductorPod, conductorRoute, helper.GetScheme()) + }) if err != nil { return ctrl.Result{}, err } - err = r.Get( - ctx, - types.NamespacedName{ - Name: conductorRoute.Name, - Namespace: conductorRoute.Namespace, + if op != controllerutil.OperationResultNone { + Log.Info(fmt.Sprintf("Route %s %s", conductorRoute.Name, op)) + } + } + if instance.Spec.GraphicalConsoles == "Enabled" { + + // + // Create the conductor pod route to enable traffic to the + // novnc service, which graphical consoles are enabled + // + conductorRouteLabels := map[string]string{ + common.AppSelector: ironic.ServiceName, + common.ComponentSelector: ironic.NoVNCComponent, + ironic.ConductorGroupSelector: ironicv1.ConductorGroupNull, + } + if instance.Spec.ConductorGroup != "" { + conductorRouteLabels[ironic.ConductorGroupSelector] = strings.ToLower(instance.Spec.ConductorGroup) + } + + desiredNoVNCRoute := ironicconductor.RouteNoVNC(conductorPod.Name, instance, conductorRouteLabels) + novncRoute := &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: desiredNoVNCRoute.Name, + Namespace: desiredNoVNCRoute.Namespace, }, - conductorRoute, - ) - if err != nil && k8s_errors.IsNotFound(err) { - Log.Info(fmt.Sprintf("Route %s does not exist, creating it", conductorRoute.Name)) - err = r.Create(ctx, conductorRoute) - if err != nil { - return ctrl.Result{}, err - } - } else { - Log.Info(fmt.Sprintf("Route %s exists, updating it", conductorRoute.Name)) - err = r.Update(ctx, conductorRoute) - if err != nil { - return ctrl.Result{}, err - } } + op, err := controllerutil.CreateOrPatch(ctx, r.Client, novncRoute, func() error { + novncRoute.Labels = desiredNoVNCRoute.Labels + novncRoute.Spec = desiredNoVNCRoute.Spec + return controllerutil.SetOwnerReference(&conductorPod, novncRoute, helper.GetScheme()) + }) + if err != nil { + return ctrl.Result{}, err + } + if op != controllerutil.OperationResultNone { + Log.Info(fmt.Sprintf("Route %s %s", novncRoute.Name, op)) + } + } } @@ -511,6 +532,36 @@ func (r *IronicConductorReconciler) reconcileNormal(ctx context.Context, instanc } } + // Roles and binding for existing service account for graphical consoles + if instance.Spec.GraphicalConsoles == "Enabled" { + // TODO: (stevebaker) Uncomment this when the role.yaml + // rule which allows namespace operations is applied. + // Until then, proceed as if the namespace has been created. + // // + // // Create the console namespace for graphical console pods + // // + // err := ensureConsoleNamespace(ctx, helper, instance.Namespace) + // if err != nil { + // return ctrl.Result{}, err + // } + + consoleNamespace := getConsoleNamespaceName(instance.Namespace) + serviceAccountName := instance.RbacResourceName() + gcRbacResult, err := reconcileGraphicalConsoleRbac( + ctx, + helper, + instance, + serviceAccountName, + consoleNamespace, + getGraphicalConsoleRbacRules(), + ) + if err != nil { + return gcRbacResult, err + } else if (gcRbacResult != ctrl.Result{}) { + return gcRbacResult, nil + } + } + // ConfigMap configMapVars := make(map[string]env.Setter) @@ -986,6 +1037,12 @@ func (r *IronicConductorReconciler) generateServiceConfigMaps( templateParameters["DefaultInspectInterface"] = getInspectInterface(instance.GetAnnotations()) templateParameters["ConductorGroup"] = instance.Spec.ConductorGroup templateParameters["LogPath"] = ironicconductor.LogPath + graphicalConsolesEnabled := instance.Spec.GraphicalConsoles == "Enabled" + templateParameters["GraphicalConsolesEnabled"] = graphicalConsolesEnabled + templateParameters["ConsoleNamespace"] = getConsoleNamespaceName(instance.Namespace) + if graphicalConsolesEnabled { + templateParameters["ConsoleImage"] = instance.Spec.ConsoleImage + } // Set GracefulShutdownTimeout for conductor pods templateParameters["GracefulShutdownTimeout"] = instance.Spec.TerminationGracePeriodSeconds @@ -1049,8 +1106,10 @@ func (r *IronicConductorReconciler) generateServiceConfigMaps( AdditionalTemplate: map[string]string{ "ironic.conf": "/common/config/ironic.conf", "01-conductor.conf": "/ironicconductor/config/01-conductor.conf", + "01-novnc.conf": "/ironicconductor/config/01-novnc.conf", "03-init-container-conductor.conf": "/ironicconductor/config/03-init-container-conductor.conf", "dnsmasq.conf": "/common/config/dnsmasq.conf", + "ironic-console-pod.yaml.template": "/ironicconductor/config/ironic-console-pod.yaml.template", }, Labels: cmLabels, }, diff --git a/internal/ironic/const.go b/internal/ironic/const.go index 9b02353e..252879dc 100644 --- a/internal/ironic/const.go +++ b/internal/ironic/const.go @@ -41,6 +41,8 @@ const ( APIComponent = "api" // InspectorComponent - InspectorComponent = "inspector" + // NoVNCComponent - + NoVNCComponent = "novnc" // ConductorGroupSelector - ConductorGroupSelector = "conductorGroup" // ImageDirectory - diff --git a/internal/ironic/initcontainer.go b/internal/ironic/initcontainer.go index 43e46c01..914f99e6 100644 --- a/internal/ironic/initcontainer.go +++ b/internal/ironic/initcontainer.go @@ -38,9 +38,11 @@ type APIDetails struct { PxeInit bool ConductorInit bool DeployHTTPURL string + NoVNCProxyURL string IngressDomain string ProvisionNetwork string ImageDirectory string + ConsoleImage string } const ( @@ -63,6 +65,7 @@ func InitContainer(init APIDetails) []corev1.Container { envVars["DatabaseHost"] = env.SetValue(init.DatabaseHost) envVars["DatabaseName"] = env.SetValue(init.DatabaseName) envVars["DeployHTTPURL"] = env.SetValue(init.DeployHTTPURL) + envVars["NoVNCProxyURL"] = env.SetValue(init.NoVNCProxyURL) envVars["IngressDomain"] = env.SetValue(init.IngressDomain) envs := []corev1.EnvVar{ @@ -201,5 +204,15 @@ func InitContainer(init APIDetails) []corev1.Container { containers = append(containers, pxeInit) } + if init.ConsoleImage != "" { + consoleImagePull := corev1.Container{ + Name: "console-image-pull", + Image: init.ConsoleImage, + SecurityContext: pod.RestrictiveSecurityContext(users.IronicUID, users.IronicGID), + Command: []string{"/bin/true"}, + } + containers = append(containers, consoleImagePull) + } + return containers } diff --git a/internal/ironicconductor/service.go b/internal/ironicconductor/service.go index 0427e961..8aeaae16 100644 --- a/internal/ironicconductor/service.go +++ b/internal/ironicconductor/service.go @@ -39,6 +39,17 @@ func Service( ports = append(ports, httpbootPort) } + // Expose the ironic-novncproxy HTTP port if graphical consoles is enabled + if instance.Spec.GraphicalConsoles == "Enabled" { + novncPort := corev1.ServicePort{ + Name: ironic.NoVNCComponent, + Port: 6090, + Protocol: corev1.ProtocolTCP, + } + ports = append(ports, novncPort) + + } + if len(ports) == 0 { return nil } @@ -80,3 +91,33 @@ func Route( }, } } + +// RouteNoVNC - Route for novnc service when graphical consoles are enabled +func RouteNoVNC( + serviceName string, + instance *ironicv1.IronicConductor, + routeLabels map[string]string, +) *routev1.Route { + serviceRef := routev1.RouteTargetReference{ + Kind: "Service", + Name: serviceName, + } + routePort := &routev1.RoutePort{ + TargetPort: intstr.FromString(ironic.NoVNCComponent), + } + return &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceName + "-novnc", + Namespace: instance.Namespace, + Labels: routeLabels, + }, + Spec: routev1.RouteSpec{ + To: serviceRef, + Port: routePort, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + } +} diff --git a/internal/ironicconductor/statefulset.go b/internal/ironicconductor/statefulset.go index f12508bb..f65c3495 100644 --- a/internal/ironicconductor/statefulset.go +++ b/internal/ironicconductor/statefulset.go @@ -80,6 +80,16 @@ func StatefulSet( PeriodSeconds: 30, InitialDelaySeconds: 5, } + novncLivenessProbe := &corev1.Probe{ + TimeoutSeconds: 10, + PeriodSeconds: 30, + InitialDelaySeconds: 5, + } + novncReadinessProbe := &corev1.Probe{ + TimeoutSeconds: 10, + PeriodSeconds: 30, + InitialDelaySeconds: 5, + } // // https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ @@ -102,6 +112,12 @@ func StatefulSet( httpbootReadinessProbe.TCPSocket = &corev1.TCPSocketAction{ Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(8088)}, } + novncLivenessProbe.TCPSocket = &corev1.TCPSocketAction{ + Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(6090)}, + } + novncReadinessProbe.TCPSocket = &corev1.TCPSocketAction{ + Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(6090)}, + } // Parse the storageRequest defined in the CR storageRequest, err := resource.ParseQuantity(instance.Spec.StorageRequest) @@ -140,6 +156,10 @@ func StatefulSet( httpbootEnvVars := map[string]env.Setter{} httpbootEnvVars["CONFIG_HASH"] = env.SetValue(configHash) + novncEnvVars := map[string]env.Setter{} + novncEnvVars["KOLLA_CONFIG_STRATEGY"] = env.SetValue("COPY_ALWAYS") + novncEnvVars["CONFIG_HASH"] = env.SetValue(configHash) + ramdiskLogsEnvVars := map[string]env.Setter{} ramdiskLogsEnvVars["CONFIG_HASH"] = env.SetValue(configHash) @@ -148,6 +168,7 @@ func StatefulSet( httpbootVolumeMounts := GetHttpbootVolumeMounts() dnsmasqVolumeMounts := GetDnsmasqVolumeMounts() ramdiskLogsVolumeMounts := GetRamdiskLogsVolumeMounts() + novncVolumeMounts := GetNoVNCVolumeMounts(instance) initVolumeMounts := GetInitVolumeMounts(instance) // Add the CA bundle @@ -158,6 +179,7 @@ func StatefulSet( dnsmasqVolumeMounts = append(dnsmasqVolumeMounts, instance.Spec.TLS.CreateVolumeMounts(nil)...) ramdiskLogsVolumeMounts = append(ramdiskLogsVolumeMounts, instance.Spec.TLS.CreateVolumeMounts(nil)...) initVolumeMounts = append(initVolumeMounts, instance.Spec.TLS.CreateVolumeMounts(nil)...) + novncVolumeMounts = append(novncVolumeMounts, instance.Spec.TLS.CreateVolumeMounts(nil)...) } resourceName := fmt.Sprintf("%s-%s", ironic.ServiceName, ironic.ConductorComponent) @@ -243,11 +265,27 @@ func StatefulSet( LivenessProbe: dnsmasqLivenessProbe, // StartupProbe: startupProbe, } - containers = []corev1.Container{ - conductorContainer, - httpbootContainer, - dnsmasqContainer, + containers = append(containers, dnsmasqContainer) + } + + if instance.Spec.GraphicalConsoles == "Enabled" { + // Only include the novnc container if graphical consoles are enabled + novncContainer := corev1.Container{ + Name: "novnc", + Command: []string{ + "/usr/bin/ironic-novncproxy", + }, + Args: []string{"--config-file", "/etc/ironic/ironic.conf", "--config-dir", "/etc/ironic/ironic.conf.d"}, + Image: instance.Spec.NoVNCProxyImage, + SecurityContext: pod.RestrictiveSecurityContext(users.IronicUID, users.IronicGID), + Env: env.MergeEnvs([]corev1.EnvVar{}, novncEnvVars), + VolumeMounts: novncVolumeMounts, + Resources: instance.Spec.Resources, + ReadinessProbe: novncReadinessProbe, + LivenessProbe: novncLivenessProbe, + // StartupProbe: startupProbe, } + containers = append(containers, novncContainer) } // Use terminationGracePeriodSeconds from CR @@ -270,7 +308,7 @@ func StatefulSet( }, Spec: corev1.PodSpec{ ServiceAccountName: instance.RbacResourceName(), - AutomountServiceAccountToken: ptr.To(false), + AutomountServiceAccountToken: ptr.To(true), // dnsmasq (added above, when present) explicitly // overrides RunAsUser/RunAsNonRoot back to root. SecurityContext: pod.RestrictivePodSecurityContext(users.IronicUID, users.IronicGID, users.ApacheGID), @@ -324,6 +362,16 @@ func StatefulSet( // Build what the fully qualified Route hostname will be when the Route exists deployHTTPURL = "http://%(PodName)s-%(PodNamespace)s.%(IngressDomain)s/" } + novncProxyURL := "" + consoleImage := "" + if instance.Spec.GraphicalConsoles == "Enabled" { + + novncProtocol := "http" + // TODO(stevebaker) detect if https should be used, and also for deployHTTPURL above + novncDomain := "%(PodName)s-novnc-%(PodNamespace)s.%(IngressDomain)s" + novncProxyURL = fmt.Sprintf("%s://%s/vnc_lite.html", novncProtocol, novncDomain) + consoleImage = instance.Spec.ConsoleImage + } initContainerDetails := ironic.APIDetails{ ContainerImage: instance.Spec.ContainerImage, @@ -339,8 +387,10 @@ func StatefulSet( PxeInit: true, ConductorInit: true, DeployHTTPURL: deployHTTPURL, + NoVNCProxyURL: novncProxyURL, IngressDomain: ingressDomain, ProvisionNetwork: instance.Spec.ProvisionNetwork, + ConsoleImage: consoleImage, } statefulset.Spec.Template.Spec.InitContainers = ironic.InitContainer(initContainerDetails) diff --git a/internal/ironicconductor/volumes.go b/internal/ironicconductor/volumes.go index a47d4fe1..b69ae18b 100644 --- a/internal/ironicconductor/volumes.go +++ b/internal/ironicconductor/volumes.go @@ -109,6 +109,12 @@ func GetConductorVolumeMounts(instance *ironicv1.IronicConductor) []corev1.Volum SubPath: "my.cnf", ReadOnly: true, }, + { + Name: "config-data", + MountPath: "/etc/ironic/ironic-console-pod.yaml.template", + SubPath: "ironic-console-pod.yaml.template", + ReadOnly: true, + }, varLibIronicVolumeMount(), } @@ -175,3 +181,40 @@ func GetDnsmasqVolumeMounts() []corev1.VolumeMount { func GetRamdiskLogsVolumeMounts() []corev1.VolumeMount { return append(ironic.GetVolumeMounts(), varLibIronicVolumeMount()) } + +// GetNoVNCVolumeMounts - NoVNC is stateless so only config is required +func GetNoVNCVolumeMounts(instance *ironicv1.IronicConductor) []corev1.VolumeMount { + vm := []corev1.VolumeMount{ + { + Name: "config-data", + MountPath: "/etc/ironic/ironic.conf", + SubPath: "ironic.conf", + ReadOnly: true, + }, + { + Name: "config-data", + MountPath: "/etc/ironic/ironic.conf.d/01-conductor.conf", + SubPath: "01-conductor.conf", + ReadOnly: true, + }, + ironic.GetMergedConfVolumeMount("/etc/ironic/ironic.conf.d/03-init-container-conductor.conf", "03-init-container-conductor.conf"), + { + Name: "config-data", + MountPath: "/etc/ironic/ironic.conf.d/04-conductor-custom.conf", + SubPath: "04-conductor-custom.conf", + ReadOnly: true, + }, + } + // 02-ironic-custom.conf is cross-cutting (shared by api/conductor/ + // inspector), stored in the parent Ironic CR's own "-config-data" + // Secret ("config-data-custom" here), not this component's own. + if ironicv1.GetOwningIronicName(instance) != "" { + vm = append(vm, corev1.VolumeMount{ + Name: "config-data-custom", + MountPath: "/etc/ironic/ironic.conf.d/02-ironic-custom.conf", + SubPath: "02-ironic-custom.conf", + ReadOnly: true, + }) + } + return append(ironic.GetVolumeMounts(), vm...) +} diff --git a/templates/common/config/ironic.conf b/templates/common/config/ironic.conf index a9e56aca..99bc8852 100644 --- a/templates/common/config/ironic.conf +++ b/templates/common/config/ironic.conf @@ -55,7 +55,6 @@ auth_strategy={{if .Standalone}}noauth{{else}}keystone{{end}} grub_config_path=EFI/BOOT/grub.cfg isolinux_bin=/usr/share/syslinux/isolinux.bin - [agent] deploy_logs_local_path=/var/lib/ironic/ramdisk-logs diff --git a/templates/ironicconductor/bin/init.sh b/templates/ironicconductor/bin/init.sh index 4a9bed9b..10bc485b 100755 --- a/templates/ironicconductor/bin/init.sh +++ b/templates/ironicconductor/bin/init.sh @@ -52,4 +52,13 @@ if [ ! -d "/var/lib/ironic/ramdisk-logs" ]; then mkdir /var/lib/ironic/ramdisk-logs fi +NOVNC_PROXY_URL=$(python3 -c ' +import os + +url_template = os.environ.get("NoVNCProxyURL", "") +if url_template: + print(url_template % os.environ) +') +crudini --set ${INIT_CONFIG} vnc public_url ${NOVNC_PROXY_URL} + echo "Conductor init successfully completed" diff --git a/templates/ironicconductor/config/01-conductor.conf b/templates/ironicconductor/config/01-conductor.conf index 9f1fbce8..bd80219d 100644 --- a/templates/ironicconductor/config/01-conductor.conf +++ b/templates/ironicconductor/config/01-conductor.conf @@ -7,6 +7,7 @@ {{- if .GracefulShutdownTimeout }} graceful_shutdown_timeout={{ .GracefulShutdownTimeout }} {{- end }} +enabled_console_interfaces={{if .GraphicalConsolesEnabled}}redfish-graphical,fake-graphical,{{end}}ipmitool-socat,no-console,fake [conductor] heartbeat_interval=20 @@ -27,3 +28,11 @@ trait_based_networking_config_file = /etc/ironic/trait_based_networking.yaml uefi_pxe_bootfile_name=bootx64.efi ipxe_bootfile_name=undionly.kpxe uefi_ipxe_bootfile_name=snponly.efi +{{if .GraphicalConsolesEnabled}} +[vnc] +enabled=True +container_provider=kubernetes +console_image={{ .ConsoleImage }} +read_only=false +kubernetes_container_template=/etc/ironic/ironic-console-pod.yaml.template +{{end}} diff --git a/templates/ironicconductor/config/01-novnc.conf b/templates/ironicconductor/config/01-novnc.conf new file mode 100644 index 00000000..23730222 --- /dev/null +++ b/templates/ironicconductor/config/01-novnc.conf @@ -0,0 +1,2 @@ +[vnc] +enabled=True diff --git a/templates/ironicconductor/config/ironic-console-pod.yaml.template b/templates/ironicconductor/config/ironic-console-pod.yaml.template new file mode 100644 index 00000000..af8bfd3e --- /dev/null +++ b/templates/ironicconductor/config/ironic-console-pod.yaml.template @@ -0,0 +1,61 @@ +apiVersion: v1 +kind: Secret +metadata: + name: "ironic-console-{{`{{ uuid }}`}}" + namespace: {{ .ConsoleNamespace }} + labels: + app: ironic + component: ironic-console + conductor: "{{`{{ conductor }}`}}" +stringData: + app-info: '{{`{{ app_info }}`}}' +--- +apiVersion: v1 +kind: Pod +metadata: + name: "ironic-console-{{`{{ uuid }}`}}" + namespace: {{ .ConsoleNamespace }} + labels: + app: ironic + component: ironic-console + conductor: "{{`{{ conductor }}`}}" +spec: + affinity: + podAffinity: + preferredDuringSchedulingIgnoredDuringExecution: + - weight: 100 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: service + operator: In + values: + - ironic + - key: component + operator: In + values: + - conductor + topologyKey: kubernetes.io/hostname + containers: + - name: x11vnc + image: "{{`{{ image }}`}}" + imagePullPolicy: IfNotPresent + ports: + - containerPort: 5900 + resources: + requests: + cpu: 250m + memory: 256Mi + limits: + cpu: 500m + memory: 1024Mi + env: + - name: APP + value: "{{`{{ app }}`}}" + - name: READ_ONLY + value: "{{`{{ read_only }}`}}" + - name: APP_INFO + valueFrom: + secretKeyRef: + name: "ironic-console-{{`{{ uuid }}`}}" + key: app-info \ No newline at end of file diff --git a/templates/ironicconductor/config/novnc-config.json b/templates/ironicconductor/config/novnc-config.json new file mode 100644 index 00000000..46b4e3f1 --- /dev/null +++ b/templates/ironicconductor/config/novnc-config.json @@ -0,0 +1,36 @@ +{ + "command": "/usr/bin/ironic-novncproxy --config-file /etc/ironic/ironic.conf --config-dir /etc/ironic/ironic.conf.d", + "config_files": [ + { + "source": "/var/lib/config-data/default/ironic.conf", + "dest": "/etc/ironic/ironic.conf", + "owner": "ironic", + "perm": "0600" + }, + { + "source": "/var/lib/config-data/default/01-novnc.conf", + "dest": "/etc/ironic/ironic.conf.d/01-novnc.conf", + "owner": "ironic", + "perm": "0600" + }, + { + "source": "/var/lib/config-data/custom/02-ironic-custom.conf", + "dest": "/etc/ironic/ironic.conf.d/02-ironic-custom.conf", + "owner": "ironic", + "perm": "0600" + }, + { + "source": "/var/lib/config-data/default/my.cnf", + "dest": "/etc/my.cnf", + "owner": "ironic", + "perm": "0644" + } + ], + "permissions": [ + { + "path": "/var/lib/ironic", + "owner": "ironic:ironic", + "recurse": true + } + ] +} diff --git a/test/functional/ironic_controller_test.go b/test/functional/ironic_controller_test.go index ca9d03c7..23734b37 100644 --- a/test/functional/ironic_controller_test.go +++ b/test/functional/ironic_controller_test.go @@ -1199,11 +1199,10 @@ var _ = Describe("Ironic controller", func() { // Ironic dbsync job g.Expect(th.GetJob(ironicNames.IronicDBSyncJobName).Spec.Template.Spec.AutomountServiceAccountToken).ToNot(BeNil()) g.Expect(*th.GetJob(ironicNames.IronicDBSyncJobName).Spec.Template.Spec.AutomountServiceAccountToken).To(BeFalse()) - // IronicConductor statefulset - get_net_ip reads from the - // DownwardAPI volume (/etc/podinfo/network-status), not - // from the k8s API, so no token is needed + // IronicConductor statefulset - needs SA token for kubectl + // access to manage pods (e.g. graphical console pods) g.Expect(th.GetStatefulSet(ironicNames.ConductorName).Spec.Template.Spec.AutomountServiceAccountToken).ToNot(BeNil()) - g.Expect(*th.GetStatefulSet(ironicNames.ConductorName).Spec.Template.Spec.AutomountServiceAccountToken).To(BeFalse()) + g.Expect(*th.GetStatefulSet(ironicNames.ConductorName).Spec.Template.Spec.AutomountServiceAccountToken).To(BeTrue()) }, timeout, interval).Should(Succeed()) }) diff --git a/test/functional/ironicconductor_controller_test.go b/test/functional/ironicconductor_controller_test.go index 8f15d7d3..2787c4fb 100644 --- a/test/functional/ironicconductor_controller_test.go +++ b/test/functional/ironicconductor_controller_test.go @@ -117,8 +117,10 @@ var _ = Describe("IronicConductor controller", func() { corev1.ConditionTrue, ) role := th.GetRole(ironicNames.ConductorRole) - Expect(role.Rules).To(HaveLen(1)) + Expect(role.Rules).To(HaveLen(3)) Expect(role.Rules[0].Resources).To(Equal([]string{"securitycontextconstraints"})) + Expect(role.Rules[1].Resources).To(Equal([]string{"pods"})) + Expect(role.Rules[2].Resources).To(Equal([]string{"secrets"})) th.ExpectCondition( ironicNames.ConductorName, ConditionGetterFunc(IronicConductorConditionGetter), @@ -759,6 +761,132 @@ var _ = Describe("IronicConductor controller", func() { ) }) + When("IronicConductor is created with graphical consoles enabled", func() { + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, + ctx, + CreateIronicSecret(ironicNames.Namespace, SecretName), + ) + DeferCleanup( + k8sClient.Delete, + ctx, + CreateMessageBusSecret(ironicNames.Namespace, MessageBusSecretName), + ) + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + ironicNames.Namespace, + "openstack", + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + DeferCleanup( + keystone.DeleteKeystoneAPI, + keystone.CreateKeystoneAPI(ironicNames.Namespace)) + spec := GetDefaultIronicConductorSpec() + spec["rpcTransport"] = "oslo" + spec["transportURLSecret"] = MessageBusSecretName + spec["graphicalConsoles"] = "Enabled" + spec["consoleImage"] = "test://console-image" + spec["novncproxyImage"] = "test://novnc-image" + DeferCleanup( + th.DeleteInstance, + CreateIronicConductor(ironicNames.ConductorName, spec)) + mariadb.CreateMariaDBDatabase(ironicNames.Namespace, ironic.DatabaseName, mariadbv1.MariaDBDatabaseSpec{}) + mariadb.SimulateMariaDBAccountCompleted(ironicNames.IronicDatabaseAccount) + mariadb.SimulateMariaDBDatabaseCompleted(ironicNames.IronicDatabaseName) + }) + + It("adds a console-image-pull init container to pre-pull the console image", func() { + Eventually(func(g Gomega) { + depl := th.GetStatefulSet(ironicNames.ConductorName) + initContainers := depl.Spec.Template.Spec.InitContainers + // Should have init, ironic-python-agent-init, pxe-init, and console-image-pull + g.Expect(initContainers).To(HaveLen(4)) + consoleInitContainer := initContainers[3] + g.Expect(consoleInitContainer.Name).To(Equal("console-image-pull")) + g.Expect(consoleInitContainer.Image).To(Equal("test://console-image")) + g.Expect(consoleInitContainer.Command).To(Equal([]string{"/bin/true"})) + }, timeout, interval).Should(Succeed()) + }) + + It("adds a novnc sidecar container", func() { + Eventually(func(g Gomega) { + depl := th.GetStatefulSet(ironicNames.ConductorName) + containers := depl.Spec.Template.Spec.Containers + // Should have ironic-conductor, httpboot, ramdisk-logs, and novnc + g.Expect(containers).To(HaveLen(4)) + novncContainer := containers[3] + g.Expect(novncContainer.Name).To(Equal("novnc")) + g.Expect(novncContainer.Image).To(Equal("test://novnc-image")) + }, timeout, interval).Should(Succeed()) + }) + }) + + When("IronicConductor is created with graphical consoles disabled", func() { + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, + ctx, + CreateIronicSecret(ironicNames.Namespace, SecretName), + ) + DeferCleanup( + k8sClient.Delete, + ctx, + CreateMessageBusSecret(ironicNames.Namespace, MessageBusSecretName), + ) + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + ironicNames.Namespace, + "openstack", + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + DeferCleanup( + keystone.DeleteKeystoneAPI, + keystone.CreateKeystoneAPI(ironicNames.Namespace)) + spec := GetDefaultIronicConductorSpec() + spec["rpcTransport"] = "oslo" + spec["transportURLSecret"] = MessageBusSecretName + DeferCleanup( + th.DeleteInstance, + CreateIronicConductor(ironicNames.ConductorName, spec)) + mariadb.CreateMariaDBDatabase(ironicNames.Namespace, ironic.DatabaseName, mariadbv1.MariaDBDatabaseSpec{}) + mariadb.SimulateMariaDBAccountCompleted(ironicNames.IronicDatabaseAccount) + mariadb.SimulateMariaDBDatabaseCompleted(ironicNames.IronicDatabaseName) + }) + + It("does not add a console-image-pull init container", func() { + Eventually(func(g Gomega) { + depl := th.GetStatefulSet(ironicNames.ConductorName) + initContainers := depl.Spec.Template.Spec.InitContainers + // Should have init, ironic-python-agent-init, pxe-init only + g.Expect(initContainers).To(HaveLen(3)) + for _, c := range initContainers { + g.Expect(c.Name).NotTo(Equal("console-image-pull")) + } + }, timeout, interval).Should(Succeed()) + }) + + It("does not add a novnc sidecar container", func() { + Eventually(func(g Gomega) { + depl := th.GetStatefulSet(ironicNames.ConductorName) + containers := depl.Spec.Template.Spec.Containers + // Should have ironic-conductor, httpboot, ramdisk-logs only + g.Expect(containers).To(HaveLen(3)) + for _, c := range containers { + g.Expect(c.Name).NotTo(Equal("novnc")) + } + }, timeout, interval).Should(Succeed()) + }) + }) + When("IronicConductor is created without TraitBasedNetworking", func() { BeforeEach(func() { DeferCleanup(