Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require EDPMServiceType #1176

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+48 −17
Open

Require EDPMServiceType #1176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apis/bases/dataplane.openstack.org_openstackdataplaneservices.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ spec:
- contents
type: object
type: object
required:
- edpmServiceType
type: object
status:
properties:
Expand Down
3 changes: 2 additions & 1 deletion apis/dataplane/v1beta1/openstackdataplaneservice_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ 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.
EDPMServiceType string `json:"edpmServiceType,omitempty" yaml:"edpmServiceType,omitempty"`
// +kubebuilder:validation:Required
EDPMServiceType string `json:"edpmServiceType" yaml:"edpmServiceType"`
}

// OpenStackDataPlaneServiceStatus defines the observed state of OpenStackDataPlaneService
Expand Down
8 changes: 0 additions & 8 deletions apis/dataplane/v1beta1/openstackdataplaneservice_webhook.go
View file
Open in desktop
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
2 changes: 2 additions & 0 deletions config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ spec:
- contents
type: object
type: object
required:
- edpmServiceType
type: object
status:
properties:
Expand Down
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_bootstrap.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_ceph_client.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_ceph_hci_pre.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_configure_network.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_configure_os.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_configure_ovs_dpdk.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_ddp_package_option.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_derive_pci_devicespec.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_download_cache.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_fips_status.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_frr.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_install_certs.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_install_os.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_libvirt.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_logging.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_dhcp.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_metadata.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_ovn.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_sriov.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_os_reboot.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_ovn.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_ovn_bgp_agent.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_run_os.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ spec:
containerImageFields:
- EdpmLogrotateCrondImage
- EdpmIscsidImage
edpmServiceType: run-os
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_ssh_known_hosts.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_swift.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ spec:
containerImageFields:
- CeilometerComputeImage
- EdpmNodeExporterImage
edpmServiceType: telemetry
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_telemetry_power_monitoring.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ spec:
containerImageFields:
- CeilometerIpmiImage
- EdpmKeplerImage
edpmServiceType: telemetry-power-monitoring
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_update.yaml
View file
Open in desktop
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
1 change: 1 addition & 0 deletions config/services/dataplane_v1beta1_openstackdataplaneservice_validate_network.yaml
View file
Open in desktop
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
View file
Open in desktop
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not actually required that edpmServiceType matches that of an existing service. It's really just if you are re-using existing ansible content from epdm-ansible, and need to re-use the same paths for cert mounts. So, I think that needs to be clarified. The field can still be required though, so that we can force the user to make that distinction when they create the service.

Copy link
Contributor Author

@bshephar bshephar Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still kind of on the fence as to whether we should do this or not. Maybe it can just be improved in documentation. But I'll note my thoughts on the topic below since we have a thread here related to it.

The only thing that concerns me with the current implementation is that it doesn't work if users try to create a new Nova service for example, but fail to specify edpmServiceType: nova. I think it probably improves the user experience by requiring the field. But then at the same time, it also puts more of a mental burden on them to now go and understand what this edpmServiceType means.

I think it's probably easier for the user if we block the CR creation by requiring this field, then they have to go and read about the parameters. Whereas the alternative is having to debug why the deployment failed, which is technically more challenging / a worse user experience.

I agree with this comment, I'm just dropping my thoughts on the thread. I'll do some rewriting of this section to include that clarification.

+
bshephar marked this conversation as resolved.
Show resolved Hide resolved
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: 5 additions & 2 deletions tests/functional/dataplane/base_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,10 @@ func DefaultDataplaneService(name types.NamespacedName) map[string]interface{} {
"namespace": name.Namespace,
},
"spec": map[string]interface{}{
"playbook": "test",
}}
"playbook": "test",
"edpmServiceType": "notGlobal",
},
}
}

// Create an empty OpenStackDataPlaneService struct
Expand All @@ -538,6 +540,7 @@ func DefaultDataplaneGlobalService(name types.NamespacedName) map[string]interfa
"spec": map[string]interface{}{
"deployOnAllNodeSets": true,
"playbook": "test",
"edpmServiceType": "global",
},
}
}
Expand Down
8 changes: 4 additions & 4 deletions tests/functional/dataplane/openstackdataplanedeployment_controller_test.go
View file
Open in desktop
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
3 changes: 2 additions & 1 deletion tests/functional/dataplane/openstackdataplanenodeset_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1236,7 +1236,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
1 change: 1 addition & 0 deletions tests/kuttl/tests/dataplane-deploy-global-service-test/00-dataplane-create.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ spec:
command: sleep 1
delegate_to: localhost
deployOnAllNodeSets: true
edpmServiceType: "custom-global-service"
---
apiVersion: v1
kind: ConfigMap
Expand Down
Loading