From 444aa4e0eb2b68251f2db925b4d8fd130a737e8c Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Wed, 1 Apr 2026 12:42:11 +1300 Subject: [PATCH 1/4] Add support for graphical consoles When the ironic spec graphicalConsoles is set to "Enabled" the following occurs: - a novncproxy container is added to the conductor pod - a route is added for users to access novncproxy URLs - conductor config is modified to enable graphical console drivers - [vnc] config enables the kubernetes container_provider Jira: OSPRH-20211 --- internal/controller/funcs.go | 5 ++ internal/controller/ironic_controller.go | 3 + .../controller/ironicconductor_controller.go | 48 ++++++++++++++++ internal/ironic/const.go | 2 + internal/ironic/initcontainer.go | 2 + internal/ironicconductor/service.go | 37 ++++++++++++ internal/ironicconductor/statefulset.go | 57 +++++++++++++++++-- templates/common/config/ironic.conf | 2 - templates/ironicconductor/bin/init.sh | 9 +++ .../ironicconductor/config/01-conductor.conf | 9 +++ .../ironicconductor/config/01-novnc.conf | 2 + .../ironicconductor/config/novnc-config.json | 36 ++++++++++++ .../ironicconductor_controller_test.go | 3 +- 13 files changed, 208 insertions(+), 7 deletions(-) create mode 100644 templates/ironicconductor/config/01-novnc.conf create mode 100644 templates/ironicconductor/config/novnc-config.json diff --git a/internal/controller/funcs.go b/internal/controller/funcs.go index 3122b407..dd7ea034 100644 --- a/internal/controller/funcs.go +++ b/internal/controller/funcs.go @@ -100,6 +100,11 @@ func getCommonRbacRules() []rbacv1.PolicyRule { Resources: []string{"pods"}, Verbs: []string{"create", "get", "list", "watch", "update", "patch", "delete"}, }, + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"create", "get", "list", "delete"}, + }, } } diff --git a/internal/controller/ironic_controller.go b/internal/controller/ironic_controller.go index aaa7f821..8a5b6925 100644 --- a/internal/controller/ironic_controller.go +++ b/internal/controller/ironic_controller.go @@ -830,6 +830,9 @@ func (r *IronicReconciler) conductorDeploymentCreateOrUpdate( Region: keystoneRegion, TLS: instance.Spec.IronicAPI.TLS.Ca, Auth: instance.Spec.Auth, + GraphicalConsoles: instance.Spec.GraphicalConsoles, + ConsoleImage: instance.Spec.Images.GraphicalConsole, + NoVNCProxyImage: instance.Spec.Images.NoVNCProxy, } if instance.Status.NotificationsURLSecret != nil { diff --git a/internal/controller/ironicconductor_controller.go b/internal/controller/ironicconductor_controller.go index c2319c5d..0018e3e9 100644 --- a/internal/controller/ironicconductor_controller.go +++ b/internal/controller/ironicconductor_controller.go @@ -475,6 +475,48 @@ func (r *IronicConductorReconciler) reconcileServices( } } } + 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) + } + + novncRoute := ironicconductor.RouteNoVNC(conductorPod.Name, instance, conductorRouteLabels) + err = controllerutil.SetOwnerReference(&conductorPod, novncRoute, helper.GetScheme()) + if err != nil { + return ctrl.Result{}, err + } + err = r.Get( + ctx, + types.NamespacedName{ + Name: novncRoute.Name, + Namespace: novncRoute.Namespace, + }, + novncRoute, + ) + if err != nil && k8s_errors.IsNotFound(err) { + Log.Info(fmt.Sprintf("Route %s does not exist, creating it", novncRoute.Name)) + err = r.Create(ctx, novncRoute) + if err != nil { + return ctrl.Result{}, err + } + } else { + Log.Info(fmt.Sprintf("Route %s exists, updating it", novncRoute.Name)) + err = r.Update(ctx, novncRoute) + if err != nil { + return ctrl.Result{}, err + } + } + + } } Log.Info("Reconciled Conductor Services successfully") @@ -967,6 +1009,11 @@ func (r *IronicConductorReconciler) generateServiceConfigMaps( templateParameters["Standalone"] = instance.Spec.Standalone templateParameters["ConductorGroup"] = instance.Spec.ConductorGroup templateParameters["LogPath"] = ironicconductor.LogPath + graphicalConsolesEnabled := instance.Spec.GraphicalConsoles == "Enabled" + templateParameters["GraphicalConsolesEnabled"] = graphicalConsolesEnabled + if graphicalConsolesEnabled { + templateParameters["ConsoleImage"] = instance.Spec.ConsoleImage + } // Set GracefulShutdownTimeout for conductor pods templateParameters["GracefulShutdownTimeout"] = instance.Spec.TerminationGracePeriodSeconds @@ -1009,6 +1056,7 @@ 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", }, diff --git a/internal/ironic/const.go b/internal/ironic/const.go index a420c57b..8acbf50a 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 0c8c90b2..ddbfe6f8 100644 --- a/internal/ironic/initcontainer.go +++ b/internal/ironic/initcontainer.go @@ -36,6 +36,7 @@ type APIDetails struct { PxeInit bool ConductorInit bool DeployHTTPURL string + NoVNCProxyURL string IngressDomain string ProvisionNetwork string ImageDirectory string @@ -57,6 +58,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{ diff --git a/internal/ironicconductor/service.go b/internal/ironicconductor/service.go index 0427e961..f214144b 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,29 @@ 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, + }, + } +} diff --git a/internal/ironicconductor/statefulset.go b/internal/ironicconductor/statefulset.go index 81dcf906..e22f096d 100644 --- a/internal/ironicconductor/statefulset.go +++ b/internal/ironicconductor/statefulset.go @@ -83,6 +83,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, + } args := []string{"-c", ServiceCommand} @@ -107,6 +117,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) @@ -148,6 +164,10 @@ func StatefulSet( httpbootEnvVars["KOLLA_CONFIG_STRATEGY"] = env.SetValue("COPY_ALWAYS") 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["KOLLA_CONFIG_STRATEGY"] = env.SetValue("COPY_ALWAYS") ramdiskLogsEnvVars["CONFIG_HASH"] = env.SetValue(configHash) @@ -155,6 +175,7 @@ func StatefulSet( volumes := GetVolumes(ctx, instance) conductorVolumeMounts := GetVolumeMounts("ironic-conductor") httpbootVolumeMounts := GetVolumeMounts("httpboot") + novncVolumeMounts := GetVolumeMounts("novnc") dnsmasqVolumeMounts := GetVolumeMounts("dnsmasq") ramdiskLogsVolumeMounts := GetVolumeMounts("ramdisk-logs") initVolumeMounts := GetInitVolumeMounts(instance) @@ -167,6 +188,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) @@ -260,11 +282,29 @@ 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{ + "/bin/bash", + }, + Args: args, + Image: instance.Spec.NoVNCProxyImage, + SecurityContext: &corev1.SecurityContext{ + RunAsUser: &runAsUser, + }, + 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 @@ -337,6 +377,14 @@ func StatefulSet( // Build what the fully qualified Route hostname will be when the Route exists deployHTTPURL = "http://%(PodName)s-%(PodNamespace)s.%(IngressDomain)s/" } + novncProxyURL := "" + 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_auto.html", novncProtocol, novncDomain) + } initContainerDetails := ironic.APIDetails{ ContainerImage: instance.Spec.ContainerImage, @@ -353,6 +401,7 @@ func StatefulSet( ConductorInit: true, Privileged: true, DeployHTTPURL: deployHTTPURL, + NoVNCProxyURL: novncProxyURL, IngressDomain: ingressDomain, ProvisionNetwork: instance.Spec.ProvisionNetwork, } diff --git a/templates/common/config/ironic.conf b/templates/common/config/ironic.conf index ceecb8d8..0119318b 100644 --- a/templates/common/config/ironic.conf +++ b/templates/common/config/ironic.conf @@ -21,7 +21,6 @@ region_name={{ .Region }} enabled_hardware_types=ipmi,idrac,irmc,fake-hardware,redfish,manual-management,ilo,ilo5 enabled_bios_interfaces=no-bios,redfish,idrac-redfish,irmc,ilo enabled_boot_interfaces=ipxe,ilo-ipxe,pxe,ilo-pxe,fake,redfish-virtual-media,idrac-redfish-virtual-media,ilo-virtual-media -enabled_console_interfaces=ipmitool-socat,ilo,no-console,fake enabled_deploy_interfaces=direct,fake,ramdisk,custom-agent default_deploy_interface=direct enabled_inspect_interfaces=inspector,no-inspect,irmc,fake,redfish,ilo @@ -55,7 +54,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 7ba097ed..95485233 100755 --- a/templates/ironicconductor/bin/init.sh +++ b/templates/ironicconductor/bin/init.sh @@ -85,4 +85,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 4dc11f64..7fa8e994 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,ilo,no-console,fake [conductor] heartbeat_interval=20 @@ -17,3 +18,11 @@ allow_provisioning_in_maintenance=false #{{- if .GracefulShutdownTimeout }} # graceful_shutdown_timeout={{ .GracefulShutdownTimeout }} #{{- end }} + +{{if .GraphicalConsolesEnabled}} +[vnc] +enabled=True +container_provider=kubernetes +console_image={{ .ConsoleImage }} +read_only=false +{{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/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/ironicconductor_controller_test.go b/test/functional/ironicconductor_controller_test.go index d76cc151..a7c26fcd 100644 --- a/test/functional/ironicconductor_controller_test.go +++ b/test/functional/ironicconductor_controller_test.go @@ -116,9 +116,10 @@ var _ = Describe("IronicConductor controller", func() { corev1.ConditionTrue, ) role := th.GetRole(ironicNames.ConductorRole) - Expect(role.Rules).To(HaveLen(2)) + 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), From 9f4a78605d935176cd522c6864988454efa7d786 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Wed, 1 Apr 2026 12:47:40 +1300 Subject: [PATCH 2/4] Use namespece openstack-ironic-consoles for pods Currently graphical console pods (and related secrets) are created in the openstack namespace. This increases security risks in 2 ways: - Ironic service account is given access to secrets in the openstack namespace - Graphical console pods share network namespace with other ironic services Putting graphical console pods improves isolation with the rest of the control plane. A future enhancement could be to create a namespace per tenant to improve isolation of graphical console pods between tenants. Jira: OSPRH-20211 --- internal/controller/funcs.go | 174 +++++++++++++++++- .../controller/ironicconductor_controller.go | 33 ++++ .../ironicconductor/config/01-conductor.conf | 1 + .../config/ironic-conductor-config.json | 6 + .../config/ironic-console-pod.yaml.template | 45 +++++ 5 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 templates/ironicconductor/config/ironic-console-pod.yaml.template diff --git a/internal/controller/funcs.go b/internal/controller/funcs.go index dd7ea034..bb75baca 100644 --- a/internal/controller/funcs.go +++ b/internal/controller/funcs.go @@ -21,21 +21,24 @@ 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" 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 @@ -88,6 +91,22 @@ var ( ) func getCommonRbacRules() []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"}, + }, + } +} + +func getGraphicalConsoleRbacRules() []rbacv1.PolicyRule { return []rbacv1.PolicyRule{ { APIGroups: []string{"security.openshift.io"}, @@ -103,7 +122,7 @@ func getCommonRbacRules() []rbacv1.PolicyRule { { APIGroups: []string{""}, Resources: []string{"secrets"}, - Verbs: []string{"create", "get", "list", "delete"}, + Verbs: []string{"create", "get", "list", "watch", "update", "patch", "delete"}, }, } } @@ -174,6 +193,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/ironicconductor_controller.go b/internal/controller/ironicconductor_controller.go index 0018e3e9..3f4192d5 100644 --- a/internal/controller/ironicconductor_controller.go +++ b/internal/controller/ironicconductor_controller.go @@ -476,6 +476,7 @@ func (r *IronicConductorReconciler) reconcileServices( } } if instance.Spec.GraphicalConsoles == "Enabled" { + // // Create the conductor pod route to enable traffic to the // novnc service, which graphical consoles are enabled @@ -552,6 +553,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) @@ -1011,6 +1042,7 @@ func (r *IronicConductorReconciler) generateServiceConfigMaps( templateParameters["LogPath"] = ironicconductor.LogPath graphicalConsolesEnabled := instance.Spec.GraphicalConsoles == "Enabled" templateParameters["GraphicalConsolesEnabled"] = graphicalConsolesEnabled + templateParameters["ConsoleNamespace"] = getConsoleNamespaceName(instance.Namespace) if graphicalConsolesEnabled { templateParameters["ConsoleImage"] = instance.Spec.ConsoleImage } @@ -1059,6 +1091,7 @@ func (r *IronicConductorReconciler) generateServiceConfigMaps( "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/templates/ironicconductor/config/01-conductor.conf b/templates/ironicconductor/config/01-conductor.conf index 7fa8e994..c2a49630 100644 --- a/templates/ironicconductor/config/01-conductor.conf +++ b/templates/ironicconductor/config/01-conductor.conf @@ -25,4 +25,5 @@ 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/ironic-conductor-config.json b/templates/ironicconductor/config/ironic-conductor-config.json index f8922626..f438efd5 100644 --- a/templates/ironicconductor/config/ironic-conductor-config.json +++ b/templates/ironicconductor/config/ironic-conductor-config.json @@ -36,6 +36,12 @@ "dest": "/etc/my.cnf", "owner": "ironic", "perm": "0644" + }, + { + "source": "/var/lib/config-data/default/ironic-console-pod.yaml.template", + "dest": "/etc/ironic/ironic-console-pod.yaml.template", + "owner": "ironic", + "perm": "0644" } ], "permissions": [ 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..3530e592 --- /dev/null +++ b/templates/ironicconductor/config/ironic-console-pod.yaml.template @@ -0,0 +1,45 @@ +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: + containers: + - name: x11vnc + image: "{{`{{ image }}`}}" + imagePullPolicy: Always + 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 From 4dacfc03b7a3bda558a89a09a50eb014a107dd12 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Wed, 1 Apr 2026 12:48:30 +1300 Subject: [PATCH 3/4] WIP Remove removed interfaces from config This removes irmc, idrac, ilo, and inspector from enabled interfaces as they have been removed upstream. This shouldn't land now but could be used as a starting point when RHOSO-19 development begins. --- templates/common/config/ironic.conf | 18 +++++++++--------- .../ironicconductor/config/01-conductor.conf | 10 +--------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/templates/common/config/ironic.conf b/templates/common/config/ironic.conf index 0119318b..18dfb222 100644 --- a/templates/common/config/ironic.conf +++ b/templates/common/config/ironic.conf @@ -18,21 +18,21 @@ region_name={{ .Region }} {{- end -}} [DEFAULT] -enabled_hardware_types=ipmi,idrac,irmc,fake-hardware,redfish,manual-management,ilo,ilo5 -enabled_bios_interfaces=no-bios,redfish,idrac-redfish,irmc,ilo -enabled_boot_interfaces=ipxe,ilo-ipxe,pxe,ilo-pxe,fake,redfish-virtual-media,idrac-redfish-virtual-media,ilo-virtual-media +enabled_hardware_types=ipmi,fake-hardware,redfish,manual-management +enabled_bios_interfaces=no-bios,redfish +enabled_boot_interfaces=ipxe,pxe,fake,redfish-virtual-media enabled_deploy_interfaces=direct,fake,ramdisk,custom-agent default_deploy_interface=direct -enabled_inspect_interfaces=inspector,no-inspect,irmc,fake,redfish,ilo -default_inspect_interface=inspector -enabled_management_interfaces=ipmitool,irmc,fake,redfish,idrac-redfish,ilo,ilo5,noop +enabled_inspect_interfaces=agent,no-inspect,fake,redfish +default_inspect_interface=agent +enabled_management_interfaces=ipmitool,fake,redfish,noop enabled_network_interfaces=flat,neutron,noop -enabled_power_interfaces=ipmitool,irmc,fake,redfish,idrac-redfish,ilo -enabled_raid_interfaces=no-raid,irmc,agent,fake,ilo5 +enabled_power_interfaces=ipmitool,fake,redfish +enabled_raid_interfaces=no-raid,agent,fake enabled_rescue_interfaces=no-rescue,agent default_rescue_interface=agent enabled_storage_interfaces=noop,fake -enabled_vendor_interfaces=no-vendor,ipmitool,idrac-redfish,redfish,ilo,fake +enabled_vendor_interfaces=no-vendor,ipmitool,redfish,fake # This is a knob to allow service role users from the service project # to have "elevated" API access to see the whole of the API surface. # https://review.opendev.org/c/openstack/ironic/+/907269 diff --git a/templates/ironicconductor/config/01-conductor.conf b/templates/ironicconductor/config/01-conductor.conf index c2a49630..7a749412 100644 --- a/templates/ironicconductor/config/01-conductor.conf +++ b/templates/ironicconductor/config/01-conductor.conf @@ -1,13 +1,5 @@ [DEFAULT] -# Default conductor configuration -# TODO: remove the below entry when we move to ironic release 2025.2 or later -# The configuration option [DEFAULT]/graceful_shutdown_timeout from oslo.service will be -# replaced by configuration option [conductor]/graceful_shutdown_timeout -# https://docs.openstack.org/releasenotes/ironic/2025.2.html#relnotes-31-0-0-stable-2025-2-upgrade-notes -{{- if .GracefulShutdownTimeout }} -graceful_shutdown_timeout={{ .GracefulShutdownTimeout }} -{{- end }} -enabled_console_interfaces={{if .GraphicalConsolesEnabled}}redfish-graphical,fake-graphical,{{end}}ipmitool-socat,ilo,no-console,fake +enabled_console_interfaces={{if .GraphicalConsolesEnabled}}redfish-graphical,fake-graphical,{{end}}ipmitool-socat,no-console,fake [conductor] heartbeat_interval=20 From 47e63f748f138e0846c97f2f5c27ee4831e2acc6 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Tue, 28 Oct 2025 17:21:04 +1300 Subject: [PATCH 4/4] DNM override new images with development images This can be removed once [1] is merged [1] https://github.com/openstack-k8s-operators/openstack-operator/pull/1633 --- internal/controller/ironic_controller.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/controller/ironic_controller.go b/internal/controller/ironic_controller.go index 8a5b6925..ec81a6ac 100644 --- a/internal/controller/ironic_controller.go +++ b/internal/controller/ironic_controller.go @@ -831,8 +831,11 @@ func (r *IronicReconciler) conductorDeploymentCreateOrUpdate( TLS: instance.Spec.IronicAPI.TLS.Ca, Auth: instance.Spec.Auth, GraphicalConsoles: instance.Spec.GraphicalConsoles, - ConsoleImage: instance.Spec.Images.GraphicalConsole, - NoVNCProxyImage: instance.Spec.Images.NoVNCProxy, + // 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/steveb/ironic-vnc-container:firefox", + NoVNCProxyImage: "quay.io/steveb/openstack-ironic-novncproxy:steveb-dev-1761687778", } if instance.Status.NotificationsURLSecret != nil {