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 Dec 3, 2024
1 parent c8ed999 commit 0782f49
Show file tree
Hide file tree
Showing 45 changed files with 61 additions and 17 deletions.
3 changes: 2 additions & 1 deletion apis/dataplane/v1beta1/openstackdataplaneservice_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type OpenstackDataPlaneServiceCert struct {
// not set, OpenStackDataPlaneService.Spec.EDPMServiceType is used. If
// OpenStackDataPlaneService.Spec.EDPMServiceType is not set, then
// OpenStackDataPlaneService.Name is used.
EDPMRoleServiceName string `json:"edpmRoleServiceName,omitempty"`
EDPMRoleServiceName string `json:"edpmRoleServiceName"`
}

// OpenStackDataPlaneServiceSpec defines the desired state of OpenStackDataPlaneService
Expand Down 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 @@ -126,6 +126,7 @@ spec:
type: array
required:
- contents
- edpmRoleServiceName
type: object
type: object
type: object
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`). If you're reusing existing edpm-ansible content in your custom service, then the `edpmServiceType` should be the same as the original service. This will ensure that your custom service has access to the TLS certificates for the service. If you create a custom `nova` service based on the edpm-ansible `nova` content, then the `edpmServiceType` should be set to `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. If your service is entirely unique and does not re-use existing edpm-ansible content, then the `edpmServiceType` should be set to the name of your custom service.
+
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 0782f49

Please sign in to comment.