From 11ae01b264d23b9f5027f7589756e5581e9ffeaa Mon Sep 17 00:00:00 2001 From: rabi Date: Thu, 10 Sep 2026 09:21:50 +0530 Subject: [PATCH] Add service dep graph, leveled execution with useParallelExecution flag - dependsOn on services; deployment plan is a topological sort into levels that run concurrently - services without dependsOn depend on their list predecessor, so existing deployments keep serial order - unresolvable dependsOn refs are skipped with a log message - build per-service AnsibleEE spec only at job start; split Deploy into startLevel/isLevelReady - useParallelExecution flag on deployment CR (default false); when not set, dep graph is not built, services run sequentially in list order and a warning is logged that dependsOn is ignored - fail fast on missing service CRs: dedupe now returns MissingServiceError instead of logging and continuing, so a misconfigured service blocks the deployment early with per-service and nodeset conditions set, instead of failing at job start Signed-off-by: rabi --- ...ack.org_openstackdataplanedeployments.yaml | 7 + ...nstack.org_openstackdataplaneservices.yaml | 11 + .../openstackdataplanedeployment_types.go | 6 + .../openstackdataplaneservice_types.go | 9 + .../v1beta1/zz_generated.deepcopy.go | 5 + bindata/crds/crds.yaml | 18 ++ ...ack.org_openstackdataplanedeployments.yaml | 7 + ...nstack.org_openstackdataplaneservices.yaml | 11 + ...1_openstackdataplaneservice_bootstrap.yaml | 3 + ...ackdataplaneservice_configure_network.yaml | 2 + ...penstackdataplaneservice_configure_os.yaml | 2 + ...nstackdataplaneservice_download_cache.yaml | 2 + ...v1beta1_openstackdataplaneservice_frr.yaml | 3 + ...enstackdataplaneservice_install_certs.yaml | 2 + ..._openstackdataplaneservice_install_os.yaml | 2 + ...ta1_openstackdataplaneservice_libvirt.yaml | 4 + ...ta1_openstackdataplaneservice_logging.yaml | 3 + ...penstackdataplaneservice_neutron_dhcp.yaml | 3 + ...tackdataplaneservice_neutron_metadata.yaml | 2 + ...openstackdataplaneservice_neutron_ovn.yaml | 2 + ...enstackdataplaneservice_neutron_sriov.yaml | 3 + ...1beta1_openstackdataplaneservice_nova.yaml | 7 + ...1_openstackdataplaneservice_os_reboot.yaml | 2 + ...v1beta1_openstackdataplaneservice_ovn.yaml | 4 + ...enstackdataplaneservice_ovn_bgp_agent.yaml | 2 + ...eta1_openstackdataplaneservice_run_os.yaml | 2 + ...stackdataplaneservice_ssh_known_hosts.yaml | 3 + ...beta1_openstackdataplaneservice_swift.yaml | 3 + ...1_openstackdataplaneservice_telemetry.yaml | 2 + ...aneservice_telemetry_power_monitoring.yaml | 2 + ...tackdataplaneservice_validate_network.yaml | 2 + ...openstackdataplanedeployment_controller.go | 76 ++++- internal/dataplane/depgraph.go | 197 ++++++++++++ internal/dataplane/depgraph_test.go | 296 ++++++++++++++++++ internal/dataplane/deployment.go | 294 +++++++++-------- internal/dataplane/service.go | 88 ++++-- test/functional/dataplane/base_test.go | 38 +++ ...tackdataplanedeployment_controller_test.go | 138 ++++++++ 38 files changed, 1107 insertions(+), 156 deletions(-) create mode 100644 internal/dataplane/depgraph.go create mode 100644 internal/dataplane/depgraph_test.go diff --git a/api/bases/dataplane.openstack.org_openstackdataplanedeployments.yaml b/api/bases/dataplane.openstack.org_openstackdataplanedeployments.yaml index d28066c53c..b5db86a344 100644 --- a/api/bases/dataplane.openstack.org_openstackdataplanedeployments.yaml +++ b/api/bases/dataplane.openstack.org_openstackdataplanedeployments.yaml @@ -116,6 +116,13 @@ spec: items: type: string type: array + useParallelExecution: + default: false + description: |- + UseParallelExecution enables parallel execution of services based on + their dependsOn dependencies. When not set (default), services run + sequentially in list order and service dependencies are ignored. + type: boolean required: - deploymentRequeueTime - nodeSets diff --git a/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml index 917bae442f..24e8d6f7dc 100644 --- a/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml +++ b/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml @@ -116,6 +116,17 @@ spec: x-kubernetes-map-type: atomic type: object type: array + dependsOn: + description: |- + DependsOn lists EDPMServiceType or CR names that must complete before + this service runs; if empty, the list predecessor is used instead. + Entries referencing a service that is not part of the deployment's + service list are ignored, so a service can be deployed on its own + (e.g. via OpenStackDataPlaneDeployment spec.servicesOverride) without + its prerequisites. + items: + type: string + type: array deployOnAllNodeSets: description: |- DeployOnAllNodeSets - should the service be deploy across all nodesets diff --git a/api/dataplane/v1beta1/openstackdataplanedeployment_types.go b/api/dataplane/v1beta1/openstackdataplanedeployment_types.go index 80758b3bc6..cea6e562be 100644 --- a/api/dataplane/v1beta1/openstackdataplanedeployment_types.go +++ b/api/dataplane/v1beta1/openstackdataplanedeployment_types.go @@ -76,6 +76,12 @@ type OpenStackDataPlaneDeploymentSpec struct { // AnsibleJobNodeSelector to target subset of worker nodes running the ansible jobs AnsibleJobNodeSelector map[string]string `json:"ansibleJobNodeSelector,omitempty"` + // UseParallelExecution enables parallel execution of services based on + // their dependsOn dependencies. When not set (default), services run + // sequentially in list order and service dependencies are ignored. + // +kubebuilder:default:=false + UseParallelExecution bool `json:"useParallelExecution,omitempty"` + // +kubebuilder:validation:MaxLength:=253 // +kubebuilder:default="openstack-aee-default-env" // AnsibleEEEnvConfigMapName is the name of the ConfigMap containing environment diff --git a/api/dataplane/v1beta1/openstackdataplaneservice_types.go b/api/dataplane/v1beta1/openstackdataplaneservice_types.go index d613f4fab6..4f77bf952b 100644 --- a/api/dataplane/v1beta1/openstackdataplaneservice_types.go +++ b/api/dataplane/v1beta1/openstackdataplaneservice_types.go @@ -117,6 +117,15 @@ type OpenStackDataPlaneServiceSpec struct { // to manage the service. If not set, will default to the // OpenStackDataPlaneService name. EDPMServiceType string `json:"edpmServiceType,omitempty" yaml:"edpmServiceType,omitempty"` + + // DependsOn lists EDPMServiceType or CR names that must complete before + // this service runs; if empty, the list predecessor is used instead. + // Entries referencing a service that is not part of the deployment's + // service list are ignored, so a service can be deployed on its own + // (e.g. via OpenStackDataPlaneDeployment spec.servicesOverride) without + // its prerequisites. + // +kubebuilder:validation:Optional + DependsOn []string `json:"dependsOn,omitempty" yaml:"dependsOn,omitempty"` } // OpenStackDataPlaneServiceStatus defines the observed state of OpenStackDataPlaneService diff --git a/api/dataplane/v1beta1/zz_generated.deepcopy.go b/api/dataplane/v1beta1/zz_generated.deepcopy.go index 476cd1ce1f..05c763fb03 100644 --- a/api/dataplane/v1beta1/zz_generated.deepcopy.go +++ b/api/dataplane/v1beta1/zz_generated.deepcopy.go @@ -829,6 +829,11 @@ func (in *OpenStackDataPlaneServiceSpec) DeepCopyInto(out *OpenStackDataPlaneSer *out = make([]string, len(*in)) copy(*out, *in) } + if in.DependsOn != nil { + in, out := &in.DependsOn, &out.DependsOn + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenStackDataPlaneServiceSpec. diff --git a/bindata/crds/crds.yaml b/bindata/crds/crds.yaml index 18f066de05..f9659aa6f1 100644 --- a/bindata/crds/crds.yaml +++ b/bindata/crds/crds.yaml @@ -20229,6 +20229,13 @@ spec: items: type: string type: array + useParallelExecution: + default: false + description: |- + UseParallelExecution enables parallel execution of services based on + their dependsOn dependencies. When not set (default), services run + sequentially in list order and service dependencies are ignored. + type: boolean required: - deploymentRequeueTime - nodeSets @@ -22177,6 +22184,17 @@ spec: x-kubernetes-map-type: atomic type: object type: array + dependsOn: + description: |- + DependsOn lists EDPMServiceType or CR names that must complete before + this service runs; if empty, the list predecessor is used instead. + Entries referencing a service that is not part of the deployment's + service list are ignored, so a service can be deployed on its own + (e.g. via OpenStackDataPlaneDeployment spec.servicesOverride) without + its prerequisites. + items: + type: string + type: array deployOnAllNodeSets: description: |- DeployOnAllNodeSets - should the service be deploy across all nodesets diff --git a/config/crd/bases/dataplane.openstack.org_openstackdataplanedeployments.yaml b/config/crd/bases/dataplane.openstack.org_openstackdataplanedeployments.yaml index d28066c53c..b5db86a344 100644 --- a/config/crd/bases/dataplane.openstack.org_openstackdataplanedeployments.yaml +++ b/config/crd/bases/dataplane.openstack.org_openstackdataplanedeployments.yaml @@ -116,6 +116,13 @@ spec: items: type: string type: array + useParallelExecution: + default: false + description: |- + UseParallelExecution enables parallel execution of services based on + their dependsOn dependencies. When not set (default), services run + sequentially in list order and service dependencies are ignored. + type: boolean required: - deploymentRequeueTime - nodeSets diff --git a/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml index 917bae442f..24e8d6f7dc 100644 --- a/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml +++ b/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml @@ -116,6 +116,17 @@ spec: x-kubernetes-map-type: atomic type: object type: array + dependsOn: + description: |- + DependsOn lists EDPMServiceType or CR names that must complete before + this service runs; if empty, the list predecessor is used instead. + Entries referencing a service that is not part of the deployment's + service list are ignored, so a service can be deployed on its own + (e.g. via OpenStackDataPlaneDeployment spec.servicesOverride) without + its prerequisites. + items: + type: string + type: array deployOnAllNodeSets: description: |- DeployOnAllNodeSets - should the service be deploy across all nodesets diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_bootstrap.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_bootstrap.yaml index d9cec33bb0..022eea874c 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_bootstrap.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_bootstrap.yaml @@ -6,3 +6,6 @@ spec: playbook: osp.edpm.bootstrap edpmServiceType: bootstrap caCerts: combined-ca-bundle + dependsOn: + - redhat + - download-cache diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_configure_network.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_configure_network.yaml index 11bfe1e86a..1811a2ac54 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_configure_network.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_configure_network.yaml @@ -5,3 +5,5 @@ metadata: spec: playbook: osp.edpm.configure_network edpmServiceType: configure-network + dependsOn: + - bootstrap diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_configure_os.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_configure_os.yaml index 6e05c430c9..df8f96e471 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_configure_os.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_configure_os.yaml @@ -5,3 +5,5 @@ metadata: spec: playbook: osp.edpm.configure_os edpmServiceType: configure-os + dependsOn: + - install-os diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_download_cache.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_download_cache.yaml index 14c0bb630e..5fd7193c5b 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_download_cache.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_download_cache.yaml @@ -5,3 +5,5 @@ metadata: spec: playbook: osp.edpm.download_cache edpmServiceType: download-cache + dependsOn: + - redhat diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_frr.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_frr.yaml index c6ac1a5031..f094145303 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_frr.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_frr.yaml @@ -4,6 +4,9 @@ metadata: name: frr spec: playbook: osp.edpm.frr + dependsOn: + - run-os + - validate-network containerImageFields: - EdpmFrrImage edpmServiceType: frr diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_install_certs.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_install_certs.yaml index e9efab000d..7481170ffb 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_install_certs.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_install_certs.yaml @@ -6,3 +6,5 @@ spec: playbook: osp.edpm.install_certs addCertMounts: True edpmServiceType: install-certs + dependsOn: + - bootstrap diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_install_os.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_install_os.yaml index e7f92632fc..63dbe2347e 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_install_os.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_install_os.yaml @@ -5,3 +5,5 @@ metadata: spec: playbook: osp.edpm.install_os edpmServiceType: install-os + dependsOn: + - bootstrap diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_libvirt.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_libvirt.yaml index 1878e0e088..1716b492f2 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_libvirt.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_libvirt.yaml @@ -4,6 +4,10 @@ metadata: name: libvirt spec: playbook: osp.edpm.libvirt + dependsOn: + - run-os + - validate-network + - install-certs dataSources: # NOTE: this Secret needs to be created before deploying the data plane. # It should contain the libvirt sasl auth password using the key LibvirtPassword diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_logging.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_logging.yaml index 9f6f23aac7..50a8bb66d5 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_logging.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_logging.yaml @@ -3,6 +3,9 @@ kind: OpenStackDataPlaneService metadata: name: logging spec: + dependsOn: + - run-os + - validate-network dataSources: - secretRef: name: logging-compute-config-data diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_dhcp.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_dhcp.yaml index 9c4b1d0091..5170b3cdc2 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_dhcp.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_dhcp.yaml @@ -4,6 +4,9 @@ metadata: name: neutron-dhcp spec: playbook: osp.edpm.neutron_dhcp + dependsOn: + - run-os + - validate-network dataSources: - secretRef: name: neutron-dhcp-agent-neutron-config diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_metadata.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_metadata.yaml index 8a2aee102c..f90862dd6a 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_metadata.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_metadata.yaml @@ -4,6 +4,8 @@ metadata: name: neutron-metadata spec: playbook: osp.edpm.neutron_metadata + dependsOn: + - ovn dataSources: - secretRef: name: neutron-ovn-metadata-agent-neutron-config diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_ovn.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_ovn.yaml index 5b570a34bb..305dd3d345 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_ovn.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_ovn.yaml @@ -4,6 +4,8 @@ metadata: name: neutron-ovn spec: playbook: osp.edpm.neutron_ovn + dependsOn: + - ovn dataSources: - secretRef: name: neutron-ovn-agent-neutron-config diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_sriov.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_sriov.yaml index af37cc7995..2518a371eb 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_sriov.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_sriov.yaml @@ -4,6 +4,9 @@ metadata: name: neutron-sriov spec: playbook: osp.edpm.neutron_sriov + dependsOn: + - run-os + - validate-network dataSources: - secretRef: name: neutron-sriov-agent-neutron-config diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_nova.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_nova.yaml index d70251b06b..5bad25c3ae 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_nova.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_nova.yaml @@ -3,6 +3,13 @@ kind: OpenStackDataPlaneService metadata: name: nova spec: + dependsOn: + - libvirt + - ssh-known-hosts + - neutron-ovn + - neutron-sriov + - neutron-dhcp + - neutron-metadata dataSources: - secretRef: name: nova-cell1-compute-config diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_os_reboot.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_os_reboot.yaml index 37abb6c903..35990d6c0d 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_os_reboot.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_os_reboot.yaml @@ -5,3 +5,5 @@ metadata: spec: playbook: osp.edpm.reboot edpmServiceType: reboot-os + dependsOn: + - run-os diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn.yaml index 7dbad97a9c..5bc83ed965 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn.yaml @@ -4,6 +4,10 @@ metadata: name: ovn spec: playbook: osp.edpm.ovn + dependsOn: + - run-os + - validate-network + - install-certs dataSources: - configMapRef: name: ovncontroller-config diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn_bgp_agent.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn_bgp_agent.yaml index 30af41db37..32d5026121 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn_bgp_agent.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn_bgp_agent.yaml @@ -4,6 +4,8 @@ metadata: name: ovn-bgp-agent spec: playbook: osp.edpm.ovn_bgp_agent + dependsOn: + - ovn dataSources: - secretRef: name: neutron-ovn-agent-neutron-config diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_run_os.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_run_os.yaml index 5a0db90bb7..53a8e75bca 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_run_os.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_run_os.yaml @@ -7,3 +7,5 @@ spec: containerImageFields: - EdpmLogrotateCrondImage edpmServiceType: run-os + dependsOn: + - configure-os diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_ssh_known_hosts.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_ssh_known_hosts.yaml index 1b38aac681..157dba1632 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_ssh_known_hosts.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_ssh_known_hosts.yaml @@ -6,3 +6,6 @@ spec: playbook: osp.edpm.ssh_known_hosts deployOnAllNodeSets: true edpmServiceType: ssh-known-hosts + dependsOn: + - install-os + - configure-network diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_swift.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_swift.yaml index 22073a43df..fee6df0247 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_swift.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_swift.yaml @@ -4,6 +4,9 @@ metadata: name: swift spec: playbook: osp.edpm.swift + dependsOn: + - run-os + - validate-network dataSources: - secretRef: name: swift-conf diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry.yaml index 9798a0630a..f061b056da 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry.yaml @@ -3,6 +3,8 @@ kind: OpenStackDataPlaneService metadata: name: telemetry spec: + dependsOn: + - nova dataSources: - secretRef: name: ceilometer-compute-config-data diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry_power_monitoring.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry_power_monitoring.yaml index 1a0a886dfb..fc6afbfe85 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry_power_monitoring.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry_power_monitoring.yaml @@ -3,6 +3,8 @@ kind: OpenStackDataPlaneService metadata: name: telemetry-power-monitoring spec: + dependsOn: + - nova dataSources: - secretRef: name: ceilometer-ipmi-config-data diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_validate_network.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_validate_network.yaml index ef8c48cb6e..1870d1c84b 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_validate_network.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_validate_network.yaml @@ -5,3 +5,5 @@ metadata: spec: playbook: osp.edpm.validate_network edpmServiceType: validate-network + dependsOn: + - configure-network diff --git a/internal/controller/dataplane/openstackdataplanedeployment_controller.go b/internal/controller/dataplane/openstackdataplanedeployment_controller.go index a738a609c0..e9bac1ac19 100644 --- a/internal/controller/dataplane/openstackdataplanedeployment_controller.go +++ b/internal/controller/dataplane/openstackdataplanedeployment_controller.go @@ -19,10 +19,12 @@ package dataplane import ( "context" + "errors" "fmt" "time" "github.com/go-playground/validator/v10" + "github.com/iancoleman/strcase" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" @@ -147,7 +149,7 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context, defer func() { // update the Ready condition based on the sub conditions // Don't update the status, if reconciler Panics if r := recover(); r != nil { - Log.Info(fmt.Sprintf("panic during reconcile %v\n", r)) + Log.Info("panic during reconcile", "panic", r) panic(r) } if instance.Status.Conditions.AllSubConditionIsTrue() { @@ -252,11 +254,42 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context, haveError := false deploymentErrMsg := "" var nodesetServiceMap map[string][]string + var serviceCache *deployment.ServiceCache backoffLimitReached := false - if nodesetServiceMap, err = deployment.DedupeServices(ctx, helper, nodeSets.Items, + if nodesetServiceMap, serviceCache, err = deployment.DedupeServices(ctx, helper, nodeSets.Items, instance.Spec.ServicesOverride); err != nil { util.LogErrorForObject(helper, err, "OpenStackDeployment error for deployment", instance) + var missingServiceErr *deployment.MissingServiceError + if errors.As(err, &missingServiceErr) { + nsConditions := instance.Status.NodeSetConditions[missingServiceErr.NodeSet] + nodeSetServiceErrorMessage := fmt.Sprintf( + dataplanev1.NodeSetServiceDeploymentErrorMessage, + missingServiceErr.Service) + " error %s" + nsConditions.MarkFalse( + dataplanev1.NodeSetDeploymentReadyCondition, + condition.ErrorReason, + condition.SeverityError, + nodeSetServiceErrorMessage, + err.Error()) + serviceCondition := condition.Type(fmt.Sprintf( + "Service%sDeploymentReady", + strcase.ToCamel(missingServiceErr.Service))) + nsConditions.MarkFalse( + serviceCondition, + condition.ErrorReason, + condition.SeverityError, + nodeSetServiceErrorMessage, + err.Error()) + instance.Status.NodeSetConditions[missingServiceErr.NodeSet] = nsConditions + instance.Status.Conditions.MarkFalse( + condition.DeploymentReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.DeploymentReadyErrorMessage, + fmt.Sprintf("nodeSet: %s error: %s", missingServiceErr.NodeSet, err.Error())) + return ctrl.Result{}, err + } instance.Status.Conditions.MarkFalse( condition.DeploymentReadyCondition, condition.ErrorReason, @@ -275,9 +308,12 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context, // The loop starts and checks NodeSet deployments sequentially. However, after they // are started, they are running in parallel, since the loop does not wait // for the first started NodeSet to finish before starting the next. + if !instance.Spec.UseParallelExecution { + Log.Info("useParallelExecution is not set to true, services will run sequentially in list order and service dependencies (dependsOn) are ignored") + } for _, nodeSet := range nodeSets.Items { - Log.Info(fmt.Sprintf("Deploying NodeSet: %s", nodeSet.Name)) + Log.Info("Deploying NodeSet", "nodeSet", nodeSet.Name) Log.Info("Set Status.Deployed to false", "instance", instance) instance.Status.Deployed = false Log.Info("Set DeploymentReadyCondition false") @@ -307,14 +343,38 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context, AeeSpec: &ansibleEESpec, InventorySecrets: globalInventorySecrets, AnsibleSSHPrivateKeySecrets: globalSSHKeySecrets, + ServiceCache: serviceCache, Version: version, } - // When ServicesOverride is set on the OpenStackDataPlaneDeployment, - // deploy those services for each OpenStackDataPlaneNodeSet. Otherwise, - // deploy with the OpenStackDataPlaneNodeSet's Services. + var serviceLevels [][]string + if instance.Spec.UseParallelExecution { + serviceLevels, err = deployment.BuildServiceLevels(ctx, helper, serviceCache, nodesetServiceMap[nodeSet.Name]) + if err != nil { + util.LogErrorForObject(helper, err, fmt.Sprintf("error building service dependency graph for NodeSet %s", nodeSet.Name), instance) + graphErrMsg := fmt.Sprintf("nodeSet: %s error: %s", nodeSet.Name, err.Error()) + nsConditions := instance.Status.NodeSetConditions[nodeSet.Name] + nsConditions.MarkFalse( + dataplanev1.NodeSetDeploymentReadyCondition, + condition.ErrorReason, + condition.SeverityError, + dataplanev1.DataPlaneNodeSetErrorMessage, + err.Error()) + instance.Status.NodeSetConditions[nodeSet.Name] = nsConditions + instance.Status.Conditions.MarkFalse( + condition.DeploymentReadyCondition, + condition.ErrorReason, + condition.SeverityError, + condition.DeploymentReadyErrorMessage, + graphErrMsg) + return ctrl.Result{}, fmt.Errorf("error building service dependency graph for nodeset %s: %w", nodeSet.Name, err) + } + } else { + serviceLevels = deployment.SerialServiceLevels(nodesetServiceMap[nodeSet.Name]) + } + var deployResult *ctrl.Result - deployResult, err = deployer.Deploy(nodesetServiceMap[nodeSet.Name]) + deployResult, err = deployer.Deploy(serviceLevels) nsConditions := instance.Status.NodeSetConditions[nodeSet.Name] nsConditions.Set(nsConditions.Mirror(dataplanev1.NodeSetDeploymentReadyCondition)) @@ -495,7 +555,7 @@ func (r *OpenStackDataPlaneDeploymentReconciler) SetupWithManager(mgr ctrl.Manag Namespace: dep.GetNamespace(), Name: dep.GetName(), } - Log.Info(fmt.Sprintf("Cert %s is used by deployment %s", obj.GetName(), dep.GetName())) + Log.Info("Cert is used by deployment", "cert", obj.GetName(), "deployment", dep.GetName()) result = append(result, reconcile.Request{NamespacedName: name}) } } diff --git a/internal/dataplane/depgraph.go b/internal/dataplane/depgraph.go new file mode 100644 index 0000000000..5146c4ca8a --- /dev/null +++ b/internal/dataplane/depgraph.go @@ -0,0 +1,197 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package deployment + +import ( + "context" + "fmt" + + "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + dataplanev1 "github.com/openstack-k8s-operators/openstack-operator/api/dataplane/v1beta1" +) + +// BuildServiceLevels topologically sorts services into parallel execution +// levels; services without dependsOn depend on their list predecessor. +func BuildServiceLevels(ctx context.Context, helper *helper.Helper, + serviceCache *ServiceCache, + services []string, +) ([][]string, error) { + + if len(services) == 0 { + return nil, nil + } + + deps, err := loadDependencies(ctx, helper, serviceCache, services) + if err != nil { + return nil, err + } + + return topoLevelSort(services, deps) +} + +// SerialServiceLevels wraps each service in its own level, preserving list +// order, for sequential execution. +func SerialServiceLevels(services []string) [][]string { + levels := make([][]string, 0, len(services)) + for _, svc := range services { + levels = append(levels, []string{svc}) + } + return levels +} + +// loadDependencies resolves dependsOn entries to CR names in the list; +// services without dependsOn get an implicit predecessor edge. +func loadDependencies(ctx context.Context, helper *helper.Helper, + serviceCache *ServiceCache, + services []string, +) (map[string][]string, error) { + + typeToName := make(map[string]string, len(services)) + nameToType := make(map[string]string, len(services)) + serviceObjects := make(map[string]dataplanev1.OpenStackDataPlaneService, len(services)) + + for _, svc := range services { + service, err := serviceCache.Get(ctx, helper, svc) + if err != nil { + return nil, err + } + serviceObjects[svc] = service + serviceType := service.Spec.EDPMServiceType + typeToName[serviceType] = svc + nameToType[svc] = serviceType + } + + deps := make(map[string][]string, len(services)) + + for i, svc := range services { + service := serviceObjects[svc] + seen := make(map[string]struct{}) + var resolved []string + addDep := func(dep string) { + if _, ok := seen[dep]; ok { + return + } + seen[dep] = struct{}{} + resolved = append(resolved, dep) + } + + if len(service.Spec.DependsOn) > 0 { + for _, dep := range service.Spec.DependsOn { + target, err := resolveDependency(dep, svc, nameToType[svc], typeToName, nameToType) + if err != nil { + return nil, err + } + if target == "" { + helper.GetLogger().Info("dependsOn entry not in nodeset service list, skipping", + "service", svc, "dependsOn", dep) + continue + } + addDep(target) + } + } else if i > 0 { + addDep(services[i-1]) + } + + deps[svc] = resolved + } + + return deps, nil +} + +// resolveDependency maps a dependsOn entry (service type or CR name) to a +// CR name in the list; unknown references resolve to "". +func resolveDependency( + dep string, + sourceSvc string, + sourceType string, + typeToName map[string]string, + nameToType map[string]string, +) (string, error) { + if _, ok := typeToName[dep]; !ok { + if resolvedType, ok := nameToType[dep]; ok { + dep = resolvedType + } + } + + if dep == sourceType { + return "", fmt.Errorf("service %q has a self-dependency", sourceSvc) + } + + if target, ok := typeToName[dep]; ok { + return target, nil + } + + return "", nil +} + +// topoLevelSort performs a Kahn-style topological sort that groups nodes +// into parallel execution levels. Returns an error on cycles. +func topoLevelSort(services []string, deps map[string][]string) ([][]string, error) { + inDegree := make(map[string]int, len(services)) + dependents := make(map[string][]string, len(services)) + + for _, svc := range services { + if _, ok := inDegree[svc]; !ok { + inDegree[svc] = 0 + } + for _, dep := range deps[svc] { + inDegree[svc]++ + dependents[dep] = append(dependents[dep], svc) + } + } + + // Seed level 0 with dependency-free services, in list order for determinism. + var current []string + for _, svc := range services { + if inDegree[svc] == 0 { + current = append(current, svc) + } + } + + var levels [][]string + visited := 0 + + for len(current) > 0 { + levels = append(levels, current) + visited += len(current) + + // Next level: newly ready services, emitted in list order for determinism. + ready := make(map[string]struct{}) + for _, svc := range current { + for _, dep := range dependents[svc] { + inDegree[dep]-- + if inDegree[dep] == 0 { + ready[dep] = struct{}{} + } + } + } + + var next []string + for _, svc := range services { + if _, ok := ready[svc]; ok { + next = append(next, svc) + } + } + current = next + } + + if visited != len(services) { + return nil, fmt.Errorf("circular dependency detected among services") + } + + return levels, nil +} diff --git a/internal/dataplane/depgraph_test.go b/internal/dataplane/depgraph_test.go new file mode 100644 index 0000000000..e7b25e1e28 --- /dev/null +++ b/internal/dataplane/depgraph_test.go @@ -0,0 +1,296 @@ +package deployment + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + dataplanev1 "github.com/openstack-k8s-operators/openstack-operator/api/dataplane/v1beta1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestTopoLevelSort(t *testing.T) { + tests := []struct { + name string + services []string + deps map[string][]string + want [][]string + wantErr string + }{ + { + name: "no deps - single level", + services: []string{"a", "b", "c"}, + deps: map[string][]string{"a": nil, "b": nil, "c": nil}, + want: [][]string{{"a", "b", "c"}}, + }, + { + name: "linear chain", + services: []string{"a", "b", "c"}, + deps: map[string][]string{"a": nil, "b": {"a"}, "c": {"b"}}, + want: [][]string{{"a"}, {"b"}, {"c"}}, + }, + { + name: "diamond", + services: []string{"a", "b", "c", "d"}, + deps: map[string][]string{"a": nil, "b": {"a"}, "c": {"a"}, "d": {"b", "c"}}, + want: [][]string{{"a"}, {"b", "c"}, {"d"}}, + }, + { + name: "multiple roots", + services: []string{"x", "y", "z", "w"}, + deps: map[string][]string{"x": nil, "y": nil, "z": {"x"}, "w": {"y"}}, + want: [][]string{{"x", "y"}, {"z", "w"}}, + }, + { + name: "single service", + services: []string{"only"}, + deps: map[string][]string{"only": nil}, + want: [][]string{{"only"}}, + }, + { + name: "preserves order within level", + services: []string{"c", "b", "a", "d"}, + deps: map[string][]string{"c": nil, "b": nil, "a": nil, "d": {"c", "b", "a"}}, + want: [][]string{{"c", "b", "a"}, {"d"}}, + }, + { + name: "complex DAG", + services: []string{"bootstrap", "configure-network", "install-os", "ovn", "libvirt", "nova"}, + deps: map[string][]string{ + "bootstrap": nil, "configure-network": {"bootstrap"}, "install-os": {"bootstrap"}, + "ovn": {"configure-network"}, "libvirt": {"install-os"}, "nova": {"ovn", "libvirt"}, + }, + want: [][]string{{"bootstrap"}, {"configure-network", "install-os"}, {"ovn", "libvirt"}, {"nova"}}, + }, + { + name: "cycle detected", + services: []string{"a", "b", "c"}, + deps: map[string][]string{"a": {"c"}, "b": {"a"}, "c": {"b"}}, + wantErr: "circular dependency", + }, + { + name: "two-node cycle", + services: []string{"a", "b"}, + deps: map[string][]string{"a": {"b"}, "b": {"a"}}, + wantErr: "circular dependency", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + levels, err := topoLevelSort(tt.services, tt.deps) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + require.Len(t, levels, len(tt.want)) + for i := range tt.want { + assert.ElementsMatch(t, tt.want[i], levels[i], "level %d", i) + } + }) + } +} + +func newTestHelper(t *testing.T, services ...*dataplanev1.OpenStackDataPlaneService) *helper.Helper { + t.Helper() + s := runtime.NewScheme() + _ = scheme.AddToScheme(s) + _ = dataplanev1.AddToScheme(s) + + var objs []client.Object + for _, svc := range services { + objs = append(objs, svc) + } + fakeClient := fakeclient.NewClientBuilder().WithScheme(s).WithObjects(objs...).Build() + + mockObj := &dataplanev1.OpenStackDataPlaneDeployment{ + ObjectMeta: metav1.ObjectMeta{Name: "test-deployment", Namespace: "openstack"}, + } + h, err := helper.NewHelper(mockObj, fakeClient, nil, s, logr.Discard()) + require.NoError(t, err) + return h +} + +func testService(name, serviceType string, dependsOn ...string) *dataplanev1.OpenStackDataPlaneService { + return &dataplanev1.OpenStackDataPlaneService{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "openstack"}, + Spec: dataplanev1.OpenStackDataPlaneServiceSpec{ + EDPMServiceType: serviceType, + DependsOn: dependsOn, + }, + } +} + +func TestLoadDependencies(t *testing.T) { + ctx := context.Background() + + t.Run("undeclared services get predecessor edge", func(t *testing.T) { + h := newTestHelper(t, + testService("a", "type-a"), + testService("b", "type-b"), + ) + deps, err := loadDependencies(ctx, h, NewServiceCache(), []string{"a", "b"}) + require.NoError(t, err) + assert.Empty(t, deps["a"]) + assert.Equal(t, []string{"a"}, deps["b"]) + }) + + t.Run("explicit deps resolve by type and CR name", func(t *testing.T) { + h := newTestHelper(t, + testService("custom-a", "type-a"), + testService("b", "type-b", "type-a", "custom-a"), + ) + deps, err := loadDependencies(ctx, h, NewServiceCache(), []string{"custom-a", "b"}) + require.NoError(t, err) + assert.Equal(t, []string{"custom-a"}, deps["b"]) + }) + + t.Run("missing deps are skipped", func(t *testing.T) { + h := newTestHelper(t, + testService("a", "type-a"), + testService("b", "type-b", "not-in-list"), + ) + deps, err := loadDependencies(ctx, h, NewServiceCache(), []string{"a", "b"}) + require.NoError(t, err) + assert.Empty(t, deps["b"]) + }) + + t.Run("self dependency errors", func(t *testing.T) { + h := newTestHelper(t, + testService("a", "type-a", "type-a"), + ) + _, err := loadDependencies(ctx, h, NewServiceCache(), []string{"a"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "self-dependency") + }) + + t.Run("self dependency via CR name errors", func(t *testing.T) { + h := newTestHelper(t, + testService("custom-a", "type-a", "custom-a"), + ) + _, err := loadDependencies(ctx, h, NewServiceCache(), []string{"custom-a"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "self-dependency") + }) +} + +func TestBuildServiceLevelsEmpty(t *testing.T) { + levels, err := BuildServiceLevels(context.Background(), nil, NewServiceCache(), nil) + require.NoError(t, err) + assert.Nil(t, levels) +} + +func TestResolveDependency(t *testing.T) { + typeToName := map[string]string{"ovn": "custom-ovn", "nova": "nova"} + nameToType := map[string]string{"custom-ovn": "ovn", "nova": "nova"} + + target, err := resolveDependency("ovn", "nova", "nova", typeToName, nameToType) + require.NoError(t, err) + assert.Equal(t, "custom-ovn", target, "should resolve by type") + + target, err = resolveDependency("custom-ovn", "nova", "nova", typeToName, nameToType) + require.NoError(t, err) + assert.Equal(t, "custom-ovn", target, "should resolve by CR name") + + target, err = resolveDependency("missing", "nova", "nova", typeToName, nameToType) + require.NoError(t, err) + assert.Equal(t, "", target, "missing dep should be skipped") + + _, err = resolveDependency("nova", "nova", "nova", typeToName, nameToType) + require.Error(t, err) + assert.Contains(t, err.Error(), "self-dependency") + + typeToName = map[string]string{"self-service": "self-service"} + nameToType = map[string]string{"self-service": "self-service"} + _, err = resolveDependency("self-service", "self-service", "self-service", typeToName, nameToType) + require.Error(t, err) + assert.Contains(t, err.Error(), "self-dependency") + + typeToName = map[string]string{"ovn": "custom-ovn"} + nameToType = map[string]string{"custom-ovn": "ovn"} + _, err = resolveDependency("custom-ovn", "custom-ovn", "ovn", typeToName, nameToType) + require.Error(t, err) + assert.Contains(t, err.Error(), "self-dependency") +} + +func TestTopoLevelSortDependencySemantics(t *testing.T) { + tests := []struct { + name string + deps map[string][]string + svcs []string + want [][]string + }{ + { + name: "fallback order forms chain", + svcs: []string{"a", "b", "c"}, + deps: map[string][]string{"a": nil, "b": {"a"}, "c": {"b"}}, + want: [][]string{{"a"}, {"b"}, {"c"}}, + }, + { + name: "explicit deps only keep multiple roots together", + svcs: []string{"b", "c", "d", "e"}, + deps: map[string][]string{"b": nil, "c": nil, "d": {"c"}, "e": {"c"}}, + want: [][]string{{"b", "c"}, {"d", "e"}}, + }, + { + name: "explicit deps do not inherit predecessor order", + svcs: []string{"a", "b", "c"}, + deps: map[string][]string{"a": nil, "b": nil, "c": {"a"}}, + want: [][]string{{"a", "b"}, {"c"}}, + }, + { + name: "explicit deps can chain independently", + svcs: []string{"a", "b", "c", "d1"}, + deps: map[string][]string{"a": nil, "b": nil, "c": {"a"}, "d1": {"c"}}, + want: [][]string{{"a", "b"}, {"c"}, {"d1"}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + levels, err := topoLevelSort(tt.svcs, tt.deps) + require.NoError(t, err) + require.Len(t, levels, len(tt.want)) + for i := range tt.want { + assert.ElementsMatch(t, tt.want[i], levels[i], "level %d", i) + } + }) + } +} + +func TestSerialServiceLevels(t *testing.T) { + tests := []struct { + name string + services []string + want [][]string + }{ + { + name: "empty", + services: nil, + want: [][]string{}, + }, + { + name: "single", + services: []string{"a"}, + want: [][]string{{"a"}}, + }, + { + name: "list order preserved", + services: []string{"a", "b", "c"}, + want: [][]string{{"a"}, {"b"}, {"c"}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + levels := SerialServiceLevels(tt.services) + require.Equal(t, tt.want, levels) + }) + } +} diff --git a/internal/dataplane/deployment.go b/internal/dataplane/deployment.go index 6b2f476cf5..b52a2b1a55 100644 --- a/internal/dataplane/deployment.go +++ b/internal/dataplane/deployment.go @@ -55,130 +55,103 @@ type Deployer struct { AeeSpec *dataplanev1.AnsibleEESpec InventorySecrets map[string]string AnsibleSSHPrivateKeySecrets map[string]string + ServiceCache *ServiceCache Version *openstackv1.OpenStackVersion + allServices []string } -// Deploy function encapsulating primary deloyment handling -func (d *Deployer) Deploy(services []string) (*ctrl.Result, error) { - log := d.Helper.GetLogger() +// Deploy runs the nodeset's leveled service plan, waiting for each level +// to be fully ready before starting the next. +func (d *Deployer) Deploy(serviceLevels [][]string) (*ctrl.Result, error) { + if d.ServiceCache == nil { + d.ServiceCache = NewServiceCache() + } - var readyCondition condition.Type - var readyMessage string - var readyWaitingMessage string - var readyErrorMessage string - var deployName string - - // Save a copy of the original ExtraMounts so it can be reset after each - // service deployment - aeeSpecMounts := make([]storage.VolMounts, len(d.AeeSpec.ExtraMounts)) - copy(aeeSpecMounts, d.AeeSpec.ExtraMounts) - // Deploy the composable services - for _, service := range services { - deployName = service - readyCondition = condition.Type(fmt.Sprintf("Service%sDeploymentReady", strcase.ToCamel(service))) - readyWaitingMessage = fmt.Sprintf(dataplanev1.NodeSetServiceDeploymentReadyWaitingMessage, deployName) - readyMessage = fmt.Sprintf(dataplanev1.NodeSetServiceDeploymentReadyMessage, deployName) - readyErrorMessage = fmt.Sprintf(dataplanev1.NodeSetServiceDeploymentErrorMessage, deployName) + " error %s" + // Flat list for cert-mount lookups + d.allServices = slices.Concat(serviceLevels...) - nsConditions := d.Status.NodeSetConditions[d.NodeSet.Name] - log.Info("Deploying service", "service", service) - foundService, err := GetService(d.Ctx, d.Helper, service) - if err != nil { - nsConditions.Set(condition.FalseCondition( - readyCondition, - condition.ErrorReason, - condition.SeverityError, - readyErrorMessage, - err.Error())) - d.Status.NodeSetConditions[d.NodeSet.Name] = nsConditions + for levelIdx, level := range serviceLevels { + d.Helper.GetLogger().Info("Deploying service level", "level", levelIdx, "services", level) + + if err := d.startLevel(levelIdx, level); err != nil { + // Level aborted; remaining services start on the next reconcile return &ctrl.Result{}, err } - containerImages := dataplaneutil.GetContainerImages(d.Version) - if containerImages.AnsibleeeImage != nil { - d.AeeSpec.OpenStackAnsibleEERunnerImage = *containerImages.AnsibleeeImage - } - if len(foundService.Spec.OpenStackAnsibleEERunnerImage) > 0 { - d.AeeSpec.OpenStackAnsibleEERunnerImage = foundService.Spec.OpenStackAnsibleEERunnerImage + if !d.isLevelReady(levelIdx, level) { + return &ctrl.Result{}, nil } + } + + return nil, nil +} - // Reset ExtraMounts to its original value, and then add in service - // specific mounts. - d.AeeSpec.ExtraMounts = make([]storage.VolMounts, len(aeeSpecMounts)) - copy(d.AeeSpec.ExtraMounts, aeeSpecMounts) - d.AeeSpec, err = d.addServiceExtraMounts(foundService) +// startLevel starts or advances every service in the level. Services that +// already completed are skipped. +func (d *Deployer) startLevel(levelIdx int, level []string) error { + log := d.Helper.GetLogger() + + for _, service := range level { + deployName := service + readyCondition := d.serviceReadyCondition(service) + readyWaitingMessage := fmt.Sprintf(dataplanev1.NodeSetServiceDeploymentReadyWaitingMessage, deployName) + readyMessage := fmt.Sprintf(dataplanev1.NodeSetServiceDeploymentReadyMessage, deployName) + readyErrorMessage := fmt.Sprintf(dataplanev1.NodeSetServiceDeploymentErrorMessage, deployName) + " error %s" + + log.Info("Deploying service", "service", service, "level", levelIdx) + foundService, err := d.ServiceCache.Get(d.Ctx, d.Helper, service) if err != nil { - nsConditions.Set(condition.FalseCondition( - readyCondition, - condition.ErrorReason, - condition.SeverityError, - readyErrorMessage, - err.Error())) - d.Status.NodeSetConditions[d.NodeSet.Name] = nsConditions - return &ctrl.Result{}, err + d.setServiceError(readyCondition, readyErrorMessage, err) + return err } - // Add certMounts - if foundService.Spec.AddCertMounts { - d.AeeSpec, err = d.addCertMounts(services) - if err != nil { - nsConditions.Set(condition.FalseCondition( - readyCondition, - condition.ErrorReason, - condition.SeverityError, - readyErrorMessage, - err.Error())) - d.Status.NodeSetConditions[d.NodeSet.Name] = nsConditions - return &ctrl.Result{}, err - } - } else if len(foundService.Spec.CACerts) > 0 { - // In general, we use the install-certs service (which has AddCertMounts=true - // to copy the cacerts over. This may not be early enough for some services - // though. In particular, we want the bootstrap service to copy the cacerts - // to the default location on the compute node. - d.AeeSpec, err = d.addCACertMount(foundService) - if err != nil { - nsConditions.Set(condition.FalseCondition( - readyCondition, - condition.ErrorReason, - condition.SeverityError, - readyErrorMessage, - err.Error())) - d.Status.NodeSetConditions[d.NodeSet.Name] = nsConditions - return &ctrl.Result{}, err - } + // Skip spec construction for services that already completed; + // their mounts were applied when the job started. + nsConditions := d.Status.NodeSetConditions[d.NodeSet.Name] + if nsConditions.IsTrue(readyCondition) { + continue } - err = d.ConditionalDeploy( + serviceAeeSpec, err := d.buildServiceAeeSpec(foundService) + if err != nil { + d.setServiceError(readyCondition, readyErrorMessage, err) + return err + } + + if err := d.ConditionalDeploy( readyCondition, readyMessage, readyWaitingMessage, readyErrorMessage, deployName, foundService, - ) - - nsConditions = d.Status.NodeSetConditions[d.NodeSet.Name] - if err != nil || !nsConditions.IsTrue(readyCondition) { - log.Info(fmt.Sprintf("Condition %s not ready", readyCondition)) - return &ctrl.Result{}, err + serviceAeeSpec, + ); err != nil { + log.Info("Condition error in service level", "condition", readyCondition, "level", levelIdx) + return err } + } - log.Info(fmt.Sprintf("Condition %s ready", readyCondition)) + return nil +} - // (TODO) Only considers the container image values from the Version - // for the time being. Can be expanded later to look at the actual - // values used from the inventory, etc. - if d.Version != nil { - vContainerImages := reflect.ValueOf(d.Version.Status.ContainerImages) - for _, cif := range foundService.Spec.ContainerImageFields { - d.Deployment.Status.ContainerImages[cif] = reflect.Indirect(vContainerImages.FieldByName(cif)).String() - } +// isLevelReady reports whether every service in the level completed. +func (d *Deployer) isLevelReady(levelIdx int, level []string) bool { + log := d.Helper.GetLogger() + nsConditions := d.Status.NodeSetConditions[d.NodeSet.Name] + for _, service := range level { + readyCondition := d.serviceReadyCondition(service) + if !nsConditions.IsTrue(readyCondition) { + log.Info("Condition not ready in service level, waiting", "condition", readyCondition, "level", levelIdx) + return false } - } + return true +} - return nil, nil +// serviceReadyCondition returns the per-service readiness condition type. +func (d *Deployer) serviceReadyCondition(service string) condition.Type { + return condition.Type(fmt.Sprintf("Service%sDeploymentReady", strcase.ToCamel(service))) } // ConditionalDeploy function encapsulating primary deloyment handling with @@ -190,15 +163,16 @@ func (d *Deployer) ConditionalDeploy( readyErrorMessage string, deployName string, foundService dataplanev1.OpenStackDataPlaneService, + aeeSpec *dataplanev1.AnsibleEESpec, ) error { var err error log := d.Helper.GetLogger() nsConditions := d.Status.NodeSetConditions[d.NodeSet.Name] if nsConditions.IsUnknown(readyCondition) { - log.Info(fmt.Sprintf("%s Unknown, starting %s", readyCondition, deployName)) + log.Info("Condition unknown, starting deploy", "condition", readyCondition, "service", deployName) err = d.DeployService( - foundService) + foundService, aeeSpec) if err != nil { util.LogErrorForObject(d.Helper, err, fmt.Sprintf("Unable to %s for %s", deployName, d.NodeSet.Name), d.NodeSet) return err @@ -219,23 +193,25 @@ func (d *Deployer) ConditionalDeploy( if err != nil { // Return nil if we don't have AnsibleEE available yet if k8s_errors.IsNotFound(err) { - log.Info(fmt.Sprintf("%s AnsibleEE job is not yet found", readyCondition)) + log.Info("AnsibleEE job not yet found", "condition", readyCondition) return nil } - log.Error(err, fmt.Sprintf("Error getting ansibleJob job for %s", deployName)) + log.Error(err, "Error getting ansibleJob job", "service", deployName) nsConditions.Set(condition.FalseCondition( readyCondition, condition.ErrorReason, condition.SeverityError, readyErrorMessage, err.Error())) + return err } if ansibleJob.Status.Succeeded > 0 { d.storeExecutionSummary(ansibleJob) - log.Info(fmt.Sprintf("Condition %s ready", readyCondition)) + log.Info("Condition ready", "condition", readyCondition) nsConditions.Set(condition.TrueCondition( readyCondition, "%s", readyMessage)) + d.recordContainerImages(foundService) } else if ansibleJob.Status.Failed > *ansibleJob.Spec.BackoffLimit { errorMsg := fmt.Sprintf("execution.name %s execution.namespace %s failed pods: %d", ansibleJob.Name, ansibleJob.Namespace, ansibleJob.Status.Failed) for _, condition := range ansibleJob.Status.Conditions { @@ -247,7 +223,7 @@ func (d *Deployer) ConditionalDeploy( errorMsg = fmt.Sprintf("backoff limit reached for execution.name %s execution.namespace %s execution.condition.message: %s", ansibleJob.Name, ansibleJob.Namespace, ansibleCondition.Message) } d.storeExecutionSummary(ansibleJob) - log.Info(fmt.Sprintf("Condition %s error", readyCondition)) + log.Info("Condition error", "condition", readyCondition) err = fmt.Errorf("%s", errorMsg) nsConditions.Set(condition.FalseCondition( readyCondition, @@ -256,7 +232,10 @@ func (d *Deployer) ConditionalDeploy( readyErrorMessage, err.Error())) } else { - log.Info(fmt.Sprintf("AnsibleEE job is not yet completed: Execution: %s, Active pods: %d, Failed pods: %d", ansibleJob.Name, ansibleJob.Status.Active, ansibleJob.Status.Failed)) + log.Info("AnsibleEE job not yet completed", + "execution", ansibleJob.Name, + "activePods", ansibleJob.Status.Active, + "failedPods", ansibleJob.Status.Failed) nsConditions.Set(condition.FalseCondition( readyCondition, condition.RequestedReason, @@ -287,25 +266,86 @@ func (d *Deployer) storeExecutionSummary(ansibleJob *batchv1.Job) { d.Status.AnsibleExecutionSummaries[ansibleJob.Name] = *summary } +// recordContainerImages captures the container image values used by a +// successfully deployed service into the deployment status. +func (d *Deployer) recordContainerImages(foundService dataplanev1.OpenStackDataPlaneService) { + // (TODO) Only considers the container image values from the Version + // for the time being. + if d.Version == nil { + return + } + vContainerImages := reflect.ValueOf(d.Version.Status.ContainerImages) + for _, cif := range foundService.Spec.ContainerImageFields { + d.Deployment.Status.ContainerImages[cif] = reflect.Indirect(vContainerImages.FieldByName(cif)).String() + } +} + +func (d *Deployer) setServiceError( + readyCondition condition.Type, + readyErrorMessage string, + err error, +) { + nsConditions := d.Status.NodeSetConditions[d.NodeSet.Name] + nsConditions.Set(condition.FalseCondition( + readyCondition, + condition.ErrorReason, + condition.SeverityError, + readyErrorMessage, + err.Error())) + d.Status.NodeSetConditions[d.NodeSet.Name] = nsConditions +} + +// buildServiceAeeSpec derives the per-service AnsibleEE spec with image +// override and mounts; called only when the service job is about to start. +func (d *Deployer) buildServiceAeeSpec( + foundService dataplanev1.OpenStackDataPlaneService, +) (*dataplanev1.AnsibleEESpec, error) { + serviceAeeSpec := d.AeeSpec.DeepCopy() + containerImages := dataplaneutil.GetContainerImages(d.Version) + if containerImages.AnsibleeeImage != nil { + serviceAeeSpec.OpenStackAnsibleEERunnerImage = *containerImages.AnsibleeeImage + } + if len(foundService.Spec.OpenStackAnsibleEERunnerImage) > 0 { + serviceAeeSpec.OpenStackAnsibleEERunnerImage = foundService.Spec.OpenStackAnsibleEERunnerImage + } + + if err := d.addServiceExtraMounts(serviceAeeSpec, foundService); err != nil { + return nil, err + } + + if foundService.Spec.AddCertMounts { + if err := d.addCertMounts(serviceAeeSpec, d.allServices); err != nil { + return nil, err + } + } else if len(foundService.Spec.CACerts) > 0 { + if err := d.addCACertMount(serviceAeeSpec, foundService); err != nil { + return nil, err + } + } + + return serviceAeeSpec, nil +} + // addCertMounts adds the cert mounts to the aeeSpec for the install-certs service func (d *Deployer) addCertMounts( + aeeSpec *dataplanev1.AnsibleEESpec, services []string, -) (*dataplanev1.AnsibleEESpec, error) { +) error { log := d.Helper.GetLogger() client := d.Helper.GetClient() for _, svc := range services { - service, err := GetService(d.Ctx, d.Helper, svc) + service, err := d.ServiceCache.Get(d.Ctx, d.Helper, svc) if err != nil { - return nil, err + return err } if service.Spec.CertsFrom != "" && service.Spec.TLSCerts == nil && service.Spec.CACerts == "" { if slices.Contains(services, service.Spec.CertsFrom) { continue } - service, err = GetService(d.Ctx, d.Helper, service.Spec.CertsFrom) + service, err = d.ServiceCache.Get(d.Ctx, d.Helper, service.Spec.CertsFrom) if err != nil { - return nil, err + return err } } @@ -313,9 +353,9 @@ func (d *Deployer) addCertMounts( if slices.Contains(services, service.Spec.EDPMServiceType) { continue } - service, err = GetService(d.Ctx, d.Helper, service.Spec.EDPMServiceType) + service, err = d.ServiceCache.Get(d.Ctx, d.Helper, service.Spec.EDPMServiceType) if err != nil { - return nil, err + return err } } @@ -336,7 +376,7 @@ func (d *Deployer) addCertMounts( certSecret := &corev1.Secret{} err := client.Get(d.Ctx, types.NamespacedName{Name: secretName, Namespace: service.Namespace}, certSecret) if err != nil { - return d.AeeSpec, err + return err } numberOfSecrets, _ := strconv.Atoi(certSecret.Labels["numberOfSecrets"]) projectedVolumeSource := corev1.ProjectedVolumeSource{ @@ -347,7 +387,7 @@ func (d *Deployer) addCertMounts( certSecret := &corev1.Secret{} err := client.Get(d.Ctx, types.NamespacedName{Name: secretName, Namespace: service.Namespace}, certSecret) if err != nil { - return d.AeeSpec, err + return err } volumeProjection := corev1.VolumeProjection{ Secret: &corev1.SecretProjection{ @@ -381,25 +421,26 @@ func (d *Deployer) addCertMounts( } volMounts.Volumes = append(volMounts.Volumes, certVolume) volMounts.Mounts = append(volMounts.Mounts, certVolumeMount) - d.AeeSpec.ExtraMounts = append(d.AeeSpec.ExtraMounts, volMounts) + aeeSpec.ExtraMounts = append(aeeSpec.ExtraMounts, volMounts) } } // add mount for cacert bundle, even if TLS-E is not enabled if len(service.Spec.CACerts) > 0 { - d.AeeSpec, err = d.addCACertMount(service) + err = d.addCACertMount(aeeSpec, service) if err != nil { - return d.AeeSpec, err + return err } } } - return d.AeeSpec, nil + return nil } func (d *Deployer) addCACertMount( + aeeSpec *dataplanev1.AnsibleEESpec, service dataplanev1.OpenStackDataPlaneService, -) (*dataplanev1.AnsibleEESpec, error) { +) error { log := d.Helper.GetLogger() client := d.Helper.GetClient() @@ -408,7 +449,7 @@ func (d *Deployer) addCACertMount( cacertSecret := &corev1.Secret{} err := client.Get(d.Ctx, types.NamespacedName{Name: service.Spec.CACerts, Namespace: service.Namespace}, cacertSecret) if err != nil { - return d.AeeSpec, err + return err } volumeName := fmt.Sprintf("%s-%s", service.Name, service.Spec.CACerts) if len(volumeName) > apimachineryvalidation.DNS1123LabelMaxLength { @@ -431,14 +472,15 @@ func (d *Deployer) addCACertMount( volMounts.Volumes = append(volMounts.Volumes, cacertVolume) volMounts.Mounts = append(volMounts.Mounts, cacertVolumeMount) - d.AeeSpec.ExtraMounts = append(d.AeeSpec.ExtraMounts, volMounts) - return d.AeeSpec, nil + aeeSpec.ExtraMounts = append(aeeSpec.ExtraMounts, volMounts) + return nil } // addServiceExtraMounts adds the service configs as ExtraMounts to aeeSpec func (d *Deployer) addServiceExtraMounts( + aeeSpec *dataplanev1.AnsibleEESpec, service dataplanev1.OpenStackDataPlaneService, -) (*dataplanev1.AnsibleEESpec, error) { +) error { baseMountPath := path.Join(ConfigPaths, service.Spec.EDPMServiceType) var configMaps []*corev1.ConfigMap @@ -447,7 +489,7 @@ func (d *Deployer) addServiceExtraMounts( for _, dataSource := range service.Spec.DataSources { _cm, _secret, err := dataplaneutil.GetDataSourceCmSecret(d.Ctx, d.Helper, service.Namespace, dataSource) if err != nil { - return nil, err + return err } if _cm != nil { @@ -505,7 +547,7 @@ func (d *Deployer) addServiceExtraMounts( } - d.AeeSpec.ExtraMounts = append(d.AeeSpec.ExtraMounts, volMounts) + aeeSpec.ExtraMounts = append(aeeSpec.ExtraMounts, volMounts) } for _, sec := range secrets { @@ -549,8 +591,8 @@ func (d *Deployer) addServiceExtraMounts( } - d.AeeSpec.ExtraMounts = append(d.AeeSpec.ExtraMounts, volMounts) + aeeSpec.ExtraMounts = append(aeeSpec.ExtraMounts, volMounts) } - return d.AeeSpec, nil + return nil } diff --git a/internal/dataplane/service.go b/internal/dataplane/service.go index 0383a95721..769b6373d7 100644 --- a/internal/dataplane/service.go +++ b/internal/dataplane/service.go @@ -44,8 +44,50 @@ type ServiceYAML struct { Spec yaml.Node } +// MissingServiceError wraps a not-found error with the nodeset and service. +type MissingServiceError struct { + NodeSet string + Service string + Err error +} + +func (e *MissingServiceError) Error() string { + return e.Err.Error() +} + +func (e *MissingServiceError) Unwrap() error { + return e.Err +} + +// ServiceCache stores reconcile-local service objects to avoid repeated API lookups. +type ServiceCache struct { + services map[string]dataplanev1.OpenStackDataPlaneService +} + +// NewServiceCache creates an empty service cache. +func NewServiceCache() *ServiceCache { + return &ServiceCache{ + services: map[string]dataplanev1.OpenStackDataPlaneService{}, + } +} + +// Get returns a cached service, fetching it on first use. +func (c *ServiceCache) Get(ctx context.Context, helper *helper.Helper, service string) (dataplanev1.OpenStackDataPlaneService, error) { + if cached, ok := c.services[service]; ok { + return cached, nil + } + + foundService, err := GetService(ctx, helper, service) + if err != nil { + return dataplanev1.OpenStackDataPlaneService{}, err + } + + c.services[service] = foundService + return foundService, nil +} + // DeployService service deployment -func (d *Deployer) DeployService(foundService dataplanev1.OpenStackDataPlaneService) error { +func (d *Deployer) DeployService(foundService dataplanev1.OpenStackDataPlaneService, aeeSpec *dataplanev1.AnsibleEESpec) error { err := dataplaneutil.AnsibleExecution( d.Ctx, d.Helper, @@ -53,10 +95,10 @@ func (d *Deployer) DeployService(foundService dataplanev1.OpenStackDataPlaneServ &foundService, d.AnsibleSSHPrivateKeySecrets, d.InventorySecrets, - d.AeeSpec, + aeeSpec, d.NodeSet) if err != nil { - d.Helper.GetLogger().Error(err, fmt.Sprintf("Unable to execute Ansible for %s", foundService.Name)) + d.Helper.GetLogger().Error(err, "Unable to execute Ansible", "service", foundService.Name) return err } @@ -152,51 +194,53 @@ func EnsureServices(ctx context.Context, helper *helper.Helper, instance *datapl return nil } -// DedupeServices deduplicates services to deploy. -// Multiple Services of same ServiceType/ServiceName in a nodeset -// Global Services in multiple NodeSets for a deployment +// DedupeServices dedupes each nodeset's services into an ordered list. func DedupeServices(ctx context.Context, helper *helper.Helper, nodesets []dataplanev1.OpenStackDataPlaneNodeSet, - serviceOverride []string) (map[string][]string, error) { - var nodeSetServiceMap = make(map[string][]string, 0) + serviceOverride []string, +) (map[string][]string, *ServiceCache, error) { + nodeSetServiceMap := make(map[string][]string) var globalServices []string var services []string var err error + serviceCache := NewServiceCache() - for _, nodeset := range nodesets { + for i := range nodesets { + nodeset := &nodesets[i] var nodeSetServices []string if len(serviceOverride) != 0 { nodeSetServices = serviceOverride } else { nodeSetServices = nodeset.Spec.Services } - services, globalServices, err = dedupe(ctx, helper, nodeSetServices, globalServices) + services, globalServices, err = dedupe(ctx, helper, serviceCache, nodeset.Name, nodeSetServices, globalServices) if err != nil { - return nil, err + return nil, nil, err } nodeSetServiceMap[nodeset.Name] = services } - helper.GetLogger().Info(fmt.Sprintf("Current Global Services %v", globalServices)) - return nodeSetServiceMap, nil + helper.GetLogger().Info("Current global services", "services", globalServices) + return nodeSetServiceMap, serviceCache, nil } func dedupe(ctx context.Context, helper *helper.Helper, + serviceCache *ServiceCache, + nodeSetName string, services []string, globalServices []string) ([]string, []string, error) { var dedupedServices []string var nodeSetServiceTypes []string updatedglobalServices := globalServices for _, svc := range services { - service, err := GetService(ctx, helper, svc) + service, err := serviceCache.Get(ctx, helper, svc) if err != nil { - if k8s_errors.IsNotFound(err) && !slices.Contains(dedupedServices, svc) { - helper.GetLogger().Error(err, fmt.Sprintf("Configured service %s does not exist", svc)) - // Add the service to the service list as it would fail later when deploying and - // Update the statuses accordingly. - dedupedServices = append(dedupedServices, svc) - continue + if !k8s_errors.IsNotFound(err) { + return dedupedServices, updatedglobalServices, err + } + return dedupedServices, updatedglobalServices, &MissingServiceError{ + NodeSet: nodeSetName, + Service: svc, + Err: err, } - return dedupedServices, updatedglobalServices, err - } serviceType := service.Spec.EDPMServiceType diff --git a/test/functional/dataplane/base_test.go b/test/functional/dataplane/base_test.go index 59204be718..7be24b7d2e 100644 --- a/test/functional/dataplane/base_test.go +++ b/test/functional/dataplane/base_test.go @@ -394,6 +394,44 @@ func SingleGlobalServiceDeploymentSpec() map[string]interface{} { } } +// Build OpenStackDataPlaneNodeSetSpec with the given services +func ParallelExecutionNodeSetSpec(nodeSetName string, services []string) map[string]interface{} { + return map[string]interface{}{ + "services": services, + "nodeTemplate": map[string]interface{}{ + "ansibleSSHPrivateKeySecret": "dataplane-ansible-ssh-private-key-secret", + "ansible": map[string]interface{}{ + "ansibleUser": "cloud-user", + }, + }, + "nodes": map[string]interface{}{ + fmt.Sprintf("%s-node-1", nodeSetName): map[string]interface{}{ + "hostName": "edpm-compute-node-1", + "networks": []infrav1.IPSetNetwork{ + {Name: "networkinternal", SubnetName: "subnet1"}, + {Name: "ctlplane", SubnetName: "subnet1"}, + }, + "ansible": map[string]interface{}{ + "ansibleHost": "192.168.122.100", + }, + }, + }, + "secretMaxSize": 1048576, + "tlsEnabled": true, + "preProvisioned": true, + } +} + +// Build OpenStackDataPlaneDeploymentSpec with useParallelExecution enabled +func ParallelExecutionDeploymentSpec() map[string]interface{} { + return map[string]interface{}{ + "nodeSets": []string{ + "edpm-compute-nodeset", + }, + "useParallelExecution": true, + } +} + func DefaultNetConfigSpec() map[string]interface{} { return map[string]interface{}{ "networks": []map[string]interface{}{{ diff --git a/test/functional/dataplane/openstackdataplanedeployment_controller_test.go b/test/functional/dataplane/openstackdataplanedeployment_controller_test.go index 489aa98bb3..b1ba9cbb14 100644 --- a/test/functional/dataplane/openstackdataplanedeployment_controller_test.go +++ b/test/functional/dataplane/openstackdataplanedeployment_controller_test.go @@ -13,7 +13,9 @@ import ( //revive:disable-next-line:dot-imports . "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers" baremetalv1 "github.com/openstack-k8s-operators/openstack-baremetal-operator/api/v1beta1" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -1929,4 +1931,140 @@ var _ = Describe("Dataplane Deployment Test", func() { }) }) + When("A dataplaneDeployment is created with useParallelExecution true and services with dependsOn", func() { + var svcAName types.NamespacedName + var svcBName types.NamespacedName + var svcCName types.NamespacedName + + BeforeEach(func() { + svcAName = types.NamespacedName{Name: "svc-a", Namespace: namespace} + svcBName = types.NamespacedName{Name: "svc-b", Namespace: namespace} + svcCName = types.NamespacedName{Name: "svc-c", Namespace: namespace} + CreateSSHSecret(dataplaneSSHSecretName) + CreateCABundleSecret(caBundleSecretName) + // svc-b and svc-c both depend on svc-a, so they form one parallel level + DeferCleanup(th.DeleteInstance, CreateDataPlaneServiceFromSpec(svcAName, map[string]interface{}{ + "edpmServiceType": "svc-a", + })) + DeferCleanup(th.DeleteInstance, CreateDataPlaneServiceFromSpec(svcBName, map[string]interface{}{ + "edpmServiceType": "svc-b", + "dependsOn": []string{"svc-a"}, + })) + DeferCleanup(th.DeleteInstance, CreateDataPlaneServiceFromSpec(svcCName, map[string]interface{}{ + "edpmServiceType": "svc-c", + "dependsOn": []string{"svc-a"}, + })) + DeferCleanup(th.DeleteInstance, CreateNetConfig(dataplaneNetConfigName, DefaultNetConfigSpec())) + DeferCleanup(th.DeleteInstance, CreateDNSMasq(dnsMasqName, DefaultDNSMasqSpec())) + SimulateDNSMasqComplete(dnsMasqName) + DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(dataplaneNodeSetName, + ParallelExecutionNodeSetSpec(dataplaneNodeSetName.Name, []string{"svc-a", "svc-b", "svc-c"}))) + DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(dataplaneDeploymentName, ParallelExecutionDeploymentSpec())) + SimulateIPSetComplete(dataplaneNodeName) + SimulateDNSDataComplete(dataplaneNodeSetName) + }) + + It("should run independent services in parallel after their dependency completes", func() { + jobName := func(service string) types.NamespacedName { + return types.NamespacedName{ + Name: fmt.Sprintf("%s-%s-%s", service, dataplaneDeploymentName.Name, dataplaneNodeSetName.Name), + Namespace: namespace, + } + } + getJob := func(name types.NamespacedName) *batchv1.Job { + job := &batchv1.Job{} + err := k8sClient.Get(ctx, name, job) + if k8s_errors.IsNotFound(err) { + return nil + } + Expect(err).NotTo(HaveOccurred()) + return job + } + completeJob := func(name types.NamespacedName) { + Eventually(func(g Gomega) { + job := GetAnsibleee(name) + job.Status.Succeeded = 1 + g.Expect(k8sClient.Status().Update(ctx, job)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + } + + // Level 0: only svc-a starts; svc-b and svc-c must wait for it + Eventually(func() *batchv1.Job { + return getJob(jobName("svc-a")) + }, timeout, interval).ShouldNot(BeNil()) + Consistently(func(g Gomega) { + g.Expect(getJob(jobName("svc-b"))).To(BeNil()) + g.Expect(getJob(jobName("svc-c"))).To(BeNil()) + }, "5s", interval).Should(Succeed()) + + // Complete svc-a; svc-b and svc-c share the next level and start together + completeJob(jobName("svc-a")) + var jobB, jobC *batchv1.Job + Eventually(func(g Gomega) { + jobB = getJob(jobName("svc-b")) + jobC = getJob(jobName("svc-c")) + g.Expect(jobB).ToNot(BeNil()) + g.Expect(jobC).ToNot(BeNil()) + }, timeout, interval).Should(Succeed()) + + // Both jobs of the parallel level exist once the dependency + // completed; the next level would not start before both do + + // Complete the parallel level; deployment becomes ready + completeJob(jobName("svc-b")) + completeJob(jobName("svc-c")) + th.ExpectCondition( + dataplaneDeploymentName, + ConditionGetterFunc(DataplaneDeploymentConditionGetter), + condition.DeploymentReadyCondition, + corev1.ConditionTrue, + ) + }) + }) + + When("A dataplaneDeployment with useParallelExecution true has circular service dependencies", func() { + var svcPName types.NamespacedName + var svcQName types.NamespacedName + + BeforeEach(func() { + svcPName = types.NamespacedName{Name: "svc-p", Namespace: namespace} + svcQName = types.NamespacedName{Name: "svc-q", Namespace: namespace} + CreateSSHSecret(dataplaneSSHSecretName) + CreateCABundleSecret(caBundleSecretName) + DeferCleanup(th.DeleteInstance, CreateDataPlaneServiceFromSpec(svcPName, map[string]interface{}{ + "edpmServiceType": "svc-p", + "dependsOn": []string{"svc-q"}, + })) + DeferCleanup(th.DeleteInstance, CreateDataPlaneServiceFromSpec(svcQName, map[string]interface{}{ + "edpmServiceType": "svc-q", + "dependsOn": []string{"svc-p"}, + })) + DeferCleanup(th.DeleteInstance, CreateNetConfig(dataplaneNetConfigName, DefaultNetConfigSpec())) + DeferCleanup(th.DeleteInstance, CreateDNSMasq(dnsMasqName, DefaultDNSMasqSpec())) + SimulateDNSMasqComplete(dnsMasqName) + DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(dataplaneNodeSetName, + ParallelExecutionNodeSetSpec(dataplaneNodeSetName.Name, []string{"svc-p", "svc-q"}))) + DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(dataplaneDeploymentName, ParallelExecutionDeploymentSpec())) + SimulateIPSetComplete(dataplaneNodeName) + SimulateDNSDataComplete(dataplaneNodeSetName) + }) + + It("should mark deployment and nodeset conditions false", func() { + th.ExpectCondition( + dataplaneDeploymentName, + ConditionGetterFunc(DataplaneDeploymentConditionGetter), + condition.DeploymentReadyCondition, + corev1.ConditionFalse, + ) + Eventually(func(g Gomega) { + deployment := GetDataplaneDeployment(dataplaneDeploymentName) + nsConditions := deployment.Status.NodeSetConditions[dataplaneNodeSetName.Name] + c := nsConditions.Get(dataplanev1.NodeSetDeploymentReadyCondition) + g.Expect(c).ToNot(BeNil()) + g.Expect(c.Status).To(Equal(corev1.ConditionFalse)) + g.Expect(c.Reason).To(Equal(condition.Reason(condition.ErrorReason))) + }, timeout, interval).Should(Succeed()) + }) + }) + })