Skip to content

Commit

Permalink
Require EDPMServiceType
Browse files Browse the repository at this point in the history
This change switches EDPMServiceType from optional to required.
Since the EDPMServiceType of custom services needs to match that of an existing service,
we also remove the defaulting mechanism in the webhook.

Signed-off-by: Brendan Shephard <[email protected]>
  • Loading branch information
bshephar committed Nov 10, 2024
1 parent 5637f8d commit a88cadf
Show file tree
Hide file tree
Showing 44 changed files with 59 additions and 16 deletions.
1 change: 1 addition & 0 deletions apis/dataplane/v1beta1/openstackdataplaneservice_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ type OpenStackDataPlaneServiceSpec struct {
// corresponds to the ansible role name (without the "edpm_" prefix) used
// to manage the service. If not set, will default to the
// OpenStackDataPlaneService name.
// +kubebuilder:validation:Required
EDPMServiceType string `json:"edpmServiceType,omitempty" yaml:"edpmServiceType,omitempty"`
}

Expand Down
8 changes: 0 additions & 8 deletions apis/dataplane/v1beta1/openstackdataplaneservice_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,9 @@ var _ webhook.Defaulter = &OpenStackDataPlaneService{}
func (r *OpenStackDataPlaneService) Default() {

openstackdataplaneservicelog.Info("default", "name", r.Name)
r.Spec.Default(r.Name)
r.DefaultLabels()
}

// Default - set defaults for this OpenStackDataPlaneService
func (spec *OpenStackDataPlaneServiceSpec) Default(name string) {
if spec.EDPMServiceType == "" {
spec.EDPMServiceType = name
}
}

// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
// +kubebuilder:webhook:path=/validate-dataplane-openstack-org-v1beta1-openstackdataplaneservice,mutating=false,failurePolicy=fail,sideEffects=None,groups=dataplane.openstack.org,resources=openstackdataplaneservices,verbs=create;update,versions=v1beta1,name=vopenstackdataplaneservice.kb.io,admissionReviewVersions=v1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: bootstrap
spec:
playbook: osp.edpm.bootstrap
edpmServiceType: bootstrap
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: ceph-client
spec:
playbook: osp.edpm.ceph_client
edpmServiceType: ceph-client
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: ceph-hci-pre
spec:
playbook: osp.edpm.ceph_hci_pre
edpmServiceType: ceph-hci-pre
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: configure-network
spec:
playbook: osp.edpm.configure_network
edpmServiceType: configure-network
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: configure-os
spec:
playbook: osp.edpm.configure_os
edpmServiceType: configure-os
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: configure-ovs-dpdk
spec:
playbook: osp.edpm.configure_ovs_dpdk
edpmServiceType: configure-ovs-dpdk
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: ddp-package-option
spec:
playbook: osp.edpm.select_kernel_ddp_package
edpmServiceType: ddp-package-option
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: derive-pci-devicespec
spec:
playbook: osp.edpm.sriov_derive_device_spec
edpmServiceType: derive-pci-devicespec
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: download-cache
spec:
playbook: osp.edpm.download_cache
edpmServiceType: download-cache
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ metadata:
spec:
label: fips-status
playbook: osp.edpm.fips_status
edpmServiceType: fips-status
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ spec:
playbook: osp.edpm.frr
containerImageFields:
- EdpmFrrImage
edpmServiceType: frr
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ metadata:
spec:
playbook: osp.edpm.install_certs
addCertMounts: True
edpmServiceType: install-certs
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: install-os
spec:
playbook: osp.edpm.install_os
edpmServiceType: install-os
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ spec:
- client auth
issuer: osp-rootca-issuer-libvirt
caCerts: combined-ca-bundle
edpmServiceType: libvirt
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ spec:
- secretRef:
name: logging-compute-config-data
playbook: osp.edpm.telemetry_logging
edpmServiceType: logging
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ spec:
caCerts: combined-ca-bundle
containerImageFields:
- EdpmNeutronDhcpAgentImage
edpmServiceType: neutron-dhcp
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ spec:
caCerts: combined-ca-bundle
containerImageFields:
- EdpmNeutronMetadataAgentImage
edpmServiceType: neutron-metadata
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ spec:
caCerts: combined-ca-bundle
containerImageFields:
- EdpmNeutronOvnAgentImage
edpmServiceType: neutron-ovn
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ spec:
caCerts: combined-ca-bundle
containerImageFields:
- EdpmNeutronSriovAgentImage
edpmServiceType: neutron-sriov
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: reboot-os
spec:
playbook: osp.edpm.reboot
edpmServiceType: reboot-os
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ spec:
caCerts: combined-ca-bundle
containerImageFields:
- OvnControllerImage
edpmServiceType: ovn
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ spec:
caCerts: combined-ca-bundle
containerImageFields:
- EdpmOvnBgpAgentImage
edpmServiceType: ovn-bgp-agent
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ spec:
containerImageFields:
- EdpmLogrotateCrondImage
- EdpmIscsidImage
edpmServiceType: run-os
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ metadata:
spec:
playbook: osp.edpm.ssh_known_hosts
deployOnAllNodeSets: true
edpmServiceType: ssh-known-hosts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ spec:
name: swift-storage-config-data
- configMapRef:
name: swift-ring-files
edpmServiceType: swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ spec:
containerImageFields:
- CeilometerComputeImage
- EdpmNodeExporterImage
edpmServiceType: telemetry
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ spec:
containerImageFields:
- CeilometerIpmiImage
- EdpmKeplerImage
edpmServiceType: telemetry-power-monitoring
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: update
spec:
playbook: osp.edpm.update
edpmServiceType: update
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ metadata:
name: validate-network
spec:
playbook: osp.edpm.validate_network
edpmServiceType: validate-network
2 changes: 1 addition & 1 deletion docs/assemblies/proc_creating-a-custom-service.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ spec:
deployOnAllNodeSets: true
----

. Optional: Specify the `edpmServiceType` field for the service. Different custom services may use the same ansible content to manage the same EDPM service (such as `ovn` or `nova`). The `DataSources`, TLS certificates, and CA certificates need to be mounted at the same locations so they can be found by the ansible content even when using a custom service. `edpmServiceType` is used to create this association. The value is the name of the default service that uses the same ansible content as the custom service. If there are multiple services with the same `edpmServiceType` listed in a nodeset or deployment spec, latter ones would be ignored.
. Required: Specify the `edpmServiceType` field for the service. Different custom services may use the same ansible content to manage the same EDPM service (such as `ovn` or `nova`). The `DataSources`, TLS certificates, and CA certificates need to be mounted at the same locations so they can be found by the ansible content even when using a custom service. `edpmServiceType` is used to create this association. The value is the name of the default service that uses the same ansible content as the custom service. If there are multiple services with the same `edpmServiceType` listed in a nodeset or deployment spec, latter ones would be ignored.
+
For example, a custom service that uses the `edpm_ovn` ansible content from `edpm-ansible` would set `edpmServiceType` to `ovn`, which matches the default `ovn` service name provided by `openstack-operator`.
+
Expand Down
7 changes: 6 additions & 1 deletion tests/functional/dataplane/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,11 @@ func DefaultDataplaneService(name types.NamespacedName) map[string]interface{} {
"metadata": map[string]interface{}{
"name": name.Name,
"namespace": name.Namespace,
}}
},
"spec": map[string]interface{}{
"edpmServiceType": "notGlobal",
},
}
}

// Create an empty OpenStackDataPlaneService struct
Expand All @@ -531,6 +535,7 @@ func DefaultDataplaneGlobalService(name types.NamespacedName) map[string]interfa
},
"spec": map[string]interface{}{
"deployOnAllNodeSets": true,
"edpmServiceType": "global",
},
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ var _ = Describe("Dataplane Deployment Test", func() {
CreateDataplaneService(dataplaneServiceName, false)
CreateDataplaneService(dataplaneGlobalServiceName, true)
CreateDataPlaneServiceFromSpec(dataplaneUpdateServiceName, map[string]interface{}{
"edpmServiceType": "foo-update-service",
"edpmServiceType": "update",
"openStackAnsibleEERunnerImage": "foo-image:latest"})

DeferCleanup(th.DeleteService, dataplaneServiceName)
Expand Down Expand Up @@ -750,7 +750,7 @@ var _ = Describe("Dataplane Deployment Test", func() {
CreateDataplaneService(dataplaneServiceName, false)
CreateDataplaneService(dataplaneGlobalServiceName, true)
CreateDataPlaneServiceFromSpec(dataplaneUpdateServiceName, map[string]interface{}{
"EDPMServiceType": "foo-update-service"})
"edpmServiceType": "foo-update-service"})

DeferCleanup(th.DeleteService, dataplaneServiceName)
DeferCleanup(th.DeleteService, dataplaneGlobalServiceName)
Expand Down Expand Up @@ -1081,7 +1081,7 @@ var _ = Describe("Dataplane Deployment Test", func() {
CreateDataplaneService(dataplaneServiceName, false)
CreateDataplaneService(dataplaneGlobalServiceName, true)
CreateDataPlaneServiceFromSpec(dataplaneUpdateServiceName, map[string]interface{}{
"EDPMServiceType": "foo-update-service"})
"edpmServiceType": "foo-update-service"})

DeferCleanup(th.DeleteService, dataplaneServiceName)
DeferCleanup(th.DeleteService, dataplaneGlobalServiceName)
Expand Down Expand Up @@ -1288,7 +1288,7 @@ var _ = Describe("Dataplane Deployment Test", func() {
CreateDataplaneService(dataplaneServiceName, false)
CreateDataplaneService(dataplaneGlobalServiceName, true)
CreateDataPlaneServiceFromSpec(dataplaneUpdateServiceName, map[string]interface{}{
"EDPMServiceType": "foo-update-service"})
"edpmServiceType": "foo-update-service"})

DeferCleanup(th.DeleteService, dataplaneServiceName)
DeferCleanup(th.DeleteService, dataplaneGlobalServiceName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,8 @@ var _ = Describe("Dataplane NodeSet Test", func() {

dataplanev1.SetupDefaults()
updateServiceSpec := map[string]interface{}{
"playbook": "osp.edpm.update",
"playbook": "osp.edpm.update",
"edpmServiceType": "update",
}
CreateDataPlaneServiceFromSpec(dataplaneUpdateServiceName, updateServiceSpec)
DeferCleanup(th.DeleteService, dataplaneUpdateServiceName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ kind: OpenStackDataPlaneService
metadata:
name: custom-global-service
spec:
edpmServiceType: sleep
label: custom-global-service
playbookContents: |
- hosts: localhost
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ kind: OpenStackDataPlaneService
metadata:
name: generic-service1
spec:
edpmServiceType: sleep
caCerts: combined-ca-bundle
tlsCerts:
default:
Expand All @@ -30,6 +31,7 @@ kind: OpenStackDataPlaneService
metadata:
name: install-certs-ovr
spec:
edpmServiceType: sleep
addCertMounts: True
playbookContents: |
- hosts: localhost
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ kind: OpenStackDataPlaneService
metadata:
name: generic-service1
spec:
edpmServiceType: sleep
caCerts: combined-ca-bundle
tlsCerts:
default:
Expand All @@ -22,6 +23,7 @@ kind: OpenStackDataPlaneService
metadata:
name: install-certs-ovr
spec:
edpmServiceType: sleep
addCertMounts: True
playbookContents: |
- hosts: localhost
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ kind: OpenStackDataPlaneService
metadata:
name: custom-svc
spec:
edpmServiceType: sleep
label: custom-svc
playbookContents: |
- hosts: localhost
Expand Down
2 changes: 2 additions & 0 deletions tests/kuttl/tests/dataplane-deploy-tls-test/00-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ kind: OpenStackDataPlaneService
metadata:
name: tls-dnsnames
spec:
edpmServiceType: sleep
caCerts: combined-ca-bundle
tlsCerts:
default:
Expand All @@ -33,6 +34,7 @@ kind: OpenStackDataPlaneService
metadata:
name: install-certs-ovrd
spec:
edpmServiceType: sleep
addCertMounts: True
playbookContents: |
- hosts: localhost
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ kind: OpenStackDataPlaneService
metadata:
name: tls-dnsnames
spec:
edpmServiceType: sleep
caCerts: combined-ca-bundle
tlsCerts:
default:
Expand All @@ -25,6 +26,7 @@ kind: OpenStackDataPlaneService
metadata:
name: install-certs-ovrd
spec:
edpmServiceType: sleep
addCertMounts: True
playbookContents: |
- hosts: localhost
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ kind: OpenStackDataPlaneService
metadata:
name: tls-dns-ips
spec:
edpmServiceType: sleep
caCerts: combined-ca-bundle
tlsCerts:
default:
Expand All @@ -27,6 +28,7 @@ kind: OpenStackDataPlaneService
metadata:
name: custom-tls-dns
spec:
edpmServiceType: sleep
caCerts: combined-ca-bundle
tlsCerts:
default:
Expand All @@ -51,6 +53,7 @@ kind: OpenStackDataPlaneService
metadata:
name: install-certs-ovrd
spec:
edpmServiceType: sleep
addCertMounts: True
playbookContents: |
- hosts: localhost
Expand Down
1 change: 1 addition & 0 deletions tests/kuttl/tests/dataplane-service-config/00-create.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ kind: OpenStackDataPlaneService
metadata:
name: kuttl-service
spec:
edpmServiceType: sleep
playbookContents: |
- hosts: localhost
gather_facts: no
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ kind: OpenStackDataPlaneService
metadata:
name: custom-img-svc
spec:
edpmServiceType: custom-image
openStackAnsibleEERunnerImage: example.com/repo/runner-image:latest
role:
playbookContents:
name: "test role"
hosts: "all"
strategy: "linear"
Expand Down

0 comments on commit a88cadf

Please sign in to comment.