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.

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

Adding general map field for configuring AEE #1102

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ spec:
type: object
spec:
properties:
ansibleEeConfig:
x-kubernetes-preserve-unknown-fields: true
ansibleExtraVars:
x-kubernetes-preserve-unknown-fields: true
ansibleJobNodeSelector:
Expand Down
3 changes: 3 additions & 0 deletions apis/dataplane/v1beta1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,4 +194,7 @@ type AnsibleEESpec struct {
// the ansible execution run with. Without specifying, it will run with
// default serviceaccount
ServiceAccountName string
// EnvConfigMapName name of configuration config map to be used
// defaults to openstack-aee-default-env
EnvConfigMapName string `json:"envConfigMapName,omitempty"`
}
6 changes: 6 additions & 0 deletions apis/dataplane/v1beta1/openstackdataplanedeployment_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ type OpenStackDataPlaneDeploymentSpec struct {
// +kubebuilder:validation:Optional
// AnsibleJobNodeSelector to target subset of worker nodes running the ansible jobs
AnsibleJobNodeSelector map[string]string `json:"ansibleJobNodeSelector,omitempty"`

// Additional configuration options for AnsibleExecutionEnvironment
// +kubebuilder:validation:Optional
// +kubebuilder:pruning:PreserveUnknownFields
// +kubebuilder:validation:Schemaless
AnsibleEEConfig DataSource `json:"ansibleEeConfig,omitempty"`
}

// OpenStackDataPlaneDeploymentStatus defines the observed state of OpenStackDataPlaneDeployment
Expand Down
1 change: 1 addition & 0 deletions apis/dataplane/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ spec:
type: object
spec:
properties:
ansibleEeConfig:
x-kubernetes-preserve-unknown-fields: true
ansibleExtraVars:
x-kubernetes-preserve-unknown-fields: true
ansibleJobNodeSelector:
Expand Down
29 changes: 29 additions & 0 deletions pkg/dataplane/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,14 @@ func (d *Deployer) Deploy(services []string) (*ctrl.Result, error) {
}
}

// set additional ansible options
// don't fail if there is a problem with struct
// since it's purely optional
err = d.setAdditionalAEEOptions()
if err != nil {
log.Error(err, fmt.Sprintf("error: %e while parsing ansibleEeConfig, defaults will be used", err))
}

err = d.ConditionalDeploy(
readyCondition,
readyMessage,
Expand Down Expand Up @@ -495,3 +503,24 @@ func (d *Deployer) addServiceExtraMounts(

return d.AeeSpec, nil
}

func (d *Deployer) setAdditionalAEEOptions() error {
var test bool
cm, _, err := dataplaneutil.GetDataSourceCmSecret(d.Ctx, d.Helper, d.NodeSet.Namespace, d.Deployment.Spec.AnsibleEEConfig)

if err != nil || cm == nil {
d.AeeSpec.EnvConfigMapName = "openstack-aee-default-env"
return fmt.Errorf("aditional AEE options not found")
}

// Check presence of the key, type and contents
configMapName, test := cm.Data["envConfigMapName"]
Copy link
Contributor

Choose a reason for hiding this comment

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

is it that the CM named d.Deployment.Spec.AnsibleEEConfig stores the name of another CM that actually stores the config? if so, why is that the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Idea is that the AnsibleEEConfig will be catch all place for all the configuration options that don't necessarily need their own field. Whether they will be defined as config maps, such as in this case, or plain values.

In this case we are storing name of a config map, which is used for environment variables. So yes, we could just remove that one level and get the same value. But it would prevent us from using this field for anything else. At some point in the future, if customer came along and asked, we would have to add another field, and another. This way we just put it in a general config map, and let this function do whatever is necessary with it.


if test && reflect.TypeOf(configMapName).Kind() == reflect.String && configMapName != "" {
d.AeeSpec.EnvConfigMapName = configMapName
} else {
d.AeeSpec.EnvConfigMapName = "openstack-aee-default-env"
}

return nil
}
3 changes: 1 addition & 2 deletions pkg/dataplane/util/ansible_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ func AnsibleExecution(
nodeSet client.Object,
) error {
var err error

executionName, labels := GetAnsibleExecutionNameAndLabels(service, deployment.GetName(), nodeSet.GetName())

existingAnsibleEE, err := GetAnsibleExecution(ctx, helper, deployment, labels)
Expand All @@ -67,7 +66,7 @@ func AnsibleExecution(
Name: executionName,
Namespace: deployment.GetNamespace(),
Labels: labels,
EnvConfigMapName: "openstack-aee-default-env",
EnvConfigMapName: aeeSpec.EnvConfigMapName,
}

ansibleEE.NetworkAttachments = aeeSpec.NetworkAttachments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1777,4 +1777,167 @@ var _ = Describe("Dataplane Deployment Test", func() {

})

When("A dataplaneDeployment is created with matching NodeSet and additional AEE options", func() {
BeforeEach(func() {
CreateSSHSecret(dataplaneSSHSecretName)
DeferCleanup(th.DeleteInstance, th.CreateSecret(neutronOvnMetadataSecretName, map[string][]byte{
"fake_keys": []byte("blih"),
}))
DeferCleanup(th.DeleteInstance, th.CreateSecret(novaNeutronMetadataSecretName, map[string][]byte{
"fake_keys": []byte("blih"),
}))
DeferCleanup(th.DeleteInstance, th.CreateSecret(novaCellComputeConfigSecretName, map[string][]byte{
"fake_keys": []byte("blih"),
}))
DeferCleanup(th.DeleteInstance, th.CreateSecret(novaMigrationSSHKey, map[string][]byte{
"ssh-privatekey": []byte("fake-ssh-private-key"),
"ssh-publickey": []byte("fake-ssh-public-key"),
}))
DeferCleanup(th.DeleteInstance, th.CreateSecret(ceilometerConfigSecretName, map[string][]byte{
"fake_keys": []byte("blih"),
}))
// DefaultDataPlanenodeSetSpec comes with three mock services
// default service
CreateDataplaneService(dataplaneServiceName, false)
// marked for deployment on all nodesets
CreateDataplaneService(dataplaneGlobalServiceName, true)
// with EDPMServiceType set
CreateDataPlaneServiceFromSpec(dataplaneUpdateServiceName, map[string]interface{}{
"edpmServiceType": "foo-update-service",
"openStackAnsibleEERunnerImage": "foo-image:latest"})
deploymentSpec := DefaultDataPlaneDeploymentSpec()
deploymentSpec["ansibleEeConfig"] = dataplanev1.DataSource{
ConfigMapRef: &dataplanev1.ConfigMapEnvSource{
LocalObjectReference: dataplanev1.LocalObjectReference{
Name: "additionalaeeonf",
},
Optional: ptr.To(true),
},
}
// Create config map for OVN service
aEEConfigMapName := types.NamespacedName{
Namespace: namespace,
Name: "additionalaeeonf",
}
mapData := map[string]interface{}{
"envConfigMapName": "test-config-map",
}
th.CreateConfigMap(aEEConfigMapName, mapData)
DeferCleanup(th.DeleteService, dataplaneServiceName)
DeferCleanup(th.DeleteService, dataplaneGlobalServiceName)
DeferCleanup(th.DeleteInstance, CreateNetConfig(dataplaneNetConfigName, DefaultNetConfigSpec()))
DeferCleanup(th.DeleteInstance, CreateDNSMasq(dnsMasqName, DefaultDNSMasqSpec()))
SimulateDNSMasqComplete(dnsMasqName)
DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(dataplaneNodeSetName, DefaultDataPlaneNodeSetSpec(dataplaneNodeSetName.Name)))
SimulateIPSetComplete(dataplaneNodeName)
SimulateDNSDataComplete(dataplaneNodeSetName)
DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(dataplaneDeploymentName, deploymentSpec))
})

It("Should have Spec fields initialized", func() {
dataplaneDeploymentInstance := GetDataplaneDeployment(dataplaneDeploymentName)
expectedSpec := dataplanev1.OpenStackDataPlaneDeploymentSpec{
NodeSets: []string{"edpm-compute-nodeset"},
AnsibleTags: "",
AnsibleLimit: "",
AnsibleSkipTags: "",
BackoffLimit: &DefaultBackoffLimit,
PreserveJobs: true,
DeploymentRequeueTime: 15,
ServicesOverride: nil,
AnsibleEEConfig: dataplanev1.DataSource{
ConfigMapRef: &dataplanev1.ConfigMapEnvSource{
LocalObjectReference: dataplanev1.LocalObjectReference{
Name: "additionalaeeonf",
},
Optional: ptr.To(true),
},
},
}
Expect(dataplaneDeploymentInstance.Spec).Should(Equal(expectedSpec))
})

It("should have conditions set", func() {

nodeSet := dataplanev1.OpenStackDataPlaneNodeSet{}
baremetal := baremetalv1.OpenStackBaremetalSet{
ObjectMeta: metav1.ObjectMeta{
Name: nodeSet.Name,
Namespace: nodeSet.Namespace,
},
}
// Create config map for OVN service
ovnConfigMapName := types.NamespacedName{
Namespace: namespace,
Name: "ovncontroller-config",
}
mapData := map[string]interface{}{
"ovsdb-config": "test-ovn-config",
}
th.CreateConfigMap(ovnConfigMapName, mapData)

nodeSet = *GetDataplaneNodeSet(dataplaneNodeSetName)

// Set baremetal provisioning conditions to True
Eventually(func(g Gomega) {
// OpenStackBaremetalSet has the same name as OpenStackDataPlaneNodeSet
g.Expect(th.K8sClient.Get(th.Ctx, dataplaneNodeSetName, &baremetal)).To(Succeed())
baremetal.Status.Conditions.MarkTrue(
condition.ReadyCondition,
condition.ReadyMessage)
g.Expect(th.K8sClient.Status().Update(th.Ctx, &baremetal)).To(Succeed())

}, th.Timeout, th.Interval).Should(Succeed())

// Create all services necessary for deployment
for _, serviceName := range nodeSet.Spec.Services {
dataplaneServiceName := types.NamespacedName{
Name: serviceName,
Namespace: namespace,
}
service := GetService(dataplaneServiceName)
deployment := GetDataplaneDeployment(dataplaneDeploymentName)
//Retrieve service AnsibleEE and set JobStatus to Successful
aeeName, _ := dataplaneutil.GetAnsibleExecutionNameAndLabels(
service, deployment.GetName(), nodeSet.GetName())
Eventually(func(g Gomega) {
// Make an AnsibleEE name for each service
ansibleeeName := types.NamespacedName{
Name: aeeName,
Namespace: dataplaneDeploymentName.Namespace,
}
ansibleEE := GetAnsibleee(ansibleeeName)

ansibleEE.Status.Succeeded = 1
g.Expect(th.K8sClient.Status().Update(th.Ctx, ansibleEE)).To(Succeed())
if service.Spec.EDPMServiceType != "" {
g.Expect(findEnvVar(ansibleEE.Spec.Template.Spec.Containers[0].Env).Value).To(ContainSubstring("edpm_service_type"))
g.Expect(findEnvVar(ansibleEE.Spec.Template.Spec.Containers[0].Env).Value).To(ContainSubstring(service.Spec.EDPMServiceType))
} else {
g.Expect(findEnvVar(ansibleEE.Spec.Template.Spec.Containers[0].Env).Value).To(ContainSubstring(serviceName))
}
if service.Spec.DeployOnAllNodeSets {
g.Expect(findEnvVar(ansibleEE.Spec.Template.Spec.Containers[0].Env).Value).To(ContainSubstring("edpm_override_hosts"))
g.Expect(findEnvVar(ansibleEE.Spec.Template.Spec.Containers[0].Env).Value).To(ContainSubstring("all"))
} else {
g.Expect(findEnvVar(ansibleEE.Spec.Template.Spec.Containers[0].Env).Value).To(ContainSubstring("edpm_override_hosts"))
g.Expect(findEnvVar(ansibleEE.Spec.Template.Spec.Containers[0].Env).Value).To(ContainSubstring(dataplaneNodeSetName.Name))
}
}, th.Timeout, th.Interval).Should(Succeed())
}

th.ExpectCondition(
dataplaneDeploymentName,
ConditionGetterFunc(DataplaneDeploymentConditionGetter),
condition.ReadyCondition,
corev1.ConditionTrue,
)
th.ExpectCondition(
dataplaneDeploymentName,
ConditionGetterFunc(DataplaneDeploymentConditionGetter),
condition.InputReadyCondition,
corev1.ConditionTrue,
)
})
})
})
Loading