Skip to content
Merged
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
9 changes: 8 additions & 1 deletion internal/controller/novacell_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -806,9 +806,16 @@ func (r *NovaCellReconciler) generateComputeConfigs(

hashes := make(map[string]env.Setter)

// Propagate RabbitMQ user names to compute-config secret so the
// openstack-operator can track which RabbitMQUser CRs are in use
extraData := map[string]string{
RabbitmqUserNameSelector: string(secret.Data[RabbitmqUserNameSelector]),
NotificationRabbitmqUserNameSelector: string(secret.Data[NotificationRabbitmqUserNameSelector]),
}

configName := instance.GetName() + "-compute-config"
err := r.GenerateConfigs(
ctx, h, instance, configName, &hashes, templateParameters, map[string]string{}, cmLabels, map[string]string{},
ctx, h, instance, configName, &hashes, templateParameters, extraData, cmLabels, map[string]string{},
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

without this PR

  $ oc get secret nova-cell1-compute-config -o json | jq -r '.data | keys'
  [
    "01-nova.conf",
    "nova-blank.conf"
  ]

with this PR - adds rabbitmq_user_name and notification_rabbitmq_user_name

  $ oc get secret nova-cell1-compute-config -o json | jq -r '.data | keys'
  [
    "01-nova.conf",
    "notification_rabbitmq_user_name",
    "nova-blank.conf",
    "rabbitmq_user_name"
  ]

if err != nil {
return err
Expand Down
13 changes: 8 additions & 5 deletions test/functional/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1063,12 +1063,15 @@ func CreateNovaWithNCellsAndEnsureReady(cellNumber int, novaNames *NovaNames) {
DeferCleanup(keystone.DeleteKeystoneAPI, novaNames.KeystoneAPIName)

// Set region on KeystoneAPI to ensure GetRegion() returns a value
keystoneAPI := keystone.GetKeystoneAPI(novaNames.KeystoneAPIName)
keystoneAPI.Spec.Region = "regionOne"
Expect(k8sClient.Update(ctx, keystoneAPI)).To(Succeed())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we might want to keep this in an Eventually bock.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

with that said i think this commit shoudl have been in a different pr

im happ to approve the priror commit but i think we had the ocnte in the eventually block becuase of ci issue in thepast with stablity on slow nodes so im concervitive with this type of refactor without review form others

// Update status separately after spec is applied
Eventually(func(g Gomega) {
keystoneAPI := keystone.GetKeystoneAPI(novaNames.KeystoneAPIName)
keystoneAPI.Spec.Region = "regionOne"
g.Expect(k8sClient.Update(ctx, keystoneAPI)).To(Succeed())
keystoneAPI.Status.Region = "regionOne"
g.Expect(k8sClient.Status().Update(ctx, keystoneAPI)).To(Succeed())
ks := keystone.GetKeystoneAPI(novaNames.KeystoneAPIName)
ks.Status.Region = "regionOne"
g.Expect(k8sClient.Status().Update(ctx, ks)).To(Succeed())
}, timeout, interval).Should(Succeed())

memcachedSpec := infra.GetDefaultMemcachedSpec()
Expand Down
20 changes: 11 additions & 9 deletions test/functional/nova_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1516,10 +1516,8 @@ var _ = Describe("Nova controller", func() {
infra.SimulateMemcachedReady(novaNames.MemcachedNamespace)
keystoneAPIName := keystone.CreateKeystoneAPI(novaNames.NovaName.Namespace)
DeferCleanup(keystone.DeleteKeystoneAPI, keystoneAPIName)
keystoneAPI := keystone.GetKeystoneAPI(keystoneAPIName)
Eventually(func(g Gomega) {
g.Expect(k8sClient.Status().Update(ctx, keystoneAPI.DeepCopy())).Should(Succeed())
}, timeout, interval).Should(Succeed())
// Note: Status update removed as it was updating with stale object and not setting any fields.
// The SimulateKeystoneServiceReady call below handles necessary setup.
keystone.SimulateKeystoneServiceReady(novaNames.KeystoneServiceName)
})
It("propagates topology to the Nova components except cell1 that has an override", func() {
Expand Down Expand Up @@ -1940,12 +1938,16 @@ var _ = Describe("Nova controller - region defaults", func() {
})

It("defaults the region to regionOne", func() {
// Clear region on KeystoneAPI spec first
keystoneAPI := keystone.GetKeystoneAPI(novaNames.KeystoneAPIName)
keystoneAPI.Spec.Region = ""
Expect(k8sClient.Update(ctx, keystoneAPI)).To(Succeed())

// Update status separately after spec is applied
Eventually(func(g Gomega) {
keystoneAPI := keystone.GetKeystoneAPI(novaNames.KeystoneAPIName)
keystoneAPI.Spec.Region = ""
g.Expect(k8sClient.Update(ctx, keystoneAPI)).To(Succeed())
keystoneAPI.Status.Region = ""
g.Expect(k8sClient.Status().Update(ctx, keystoneAPI)).To(Succeed())
ks := keystone.GetKeystoneAPI(novaNames.KeystoneAPIName)
ks.Status.Region = ""
g.Expect(k8sClient.Status().Update(ctx, ks)).To(Succeed())
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
Expand Down
6 changes: 2 additions & 4 deletions test/functional/nova_multicell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -889,10 +889,8 @@ var _ = Describe("Nova multi cell", func() {
infra.SimulateMemcachedReady(novaNames.MemcachedNamespace)
keystoneAPIName := keystone.CreateKeystoneAPI(novaNames.NovaName.Namespace)
DeferCleanup(keystone.DeleteKeystoneAPI, keystoneAPIName)
keystoneAPI := keystone.GetKeystoneAPI(keystoneAPIName)
Eventually(func(g Gomega) {
g.Expect(k8sClient.Status().Update(ctx, keystoneAPI.DeepCopy())).Should(Succeed())
}, timeout, interval).Should(Succeed())
// Note: Status update removed as it was updating with stale object and not setting any fields.
// The SimulateKeystoneServiceReady call below handles necessary setup.
keystone.SimulateKeystoneServiceReady(novaNames.KeystoneServiceName)
})
It("cell0 becomes ready without metadata and the rest of nova is deployed", func() {
Expand Down
83 changes: 43 additions & 40 deletions test/functional/nova_scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,51 +285,54 @@ var _ = Describe("NovaScheduler controller", func() {
)

// Wait for config to be regenerated with region
var configData string
Eventually(func(g Gomega) {
configDataMap := th.GetSecret(novaNames.SchedulerConfigDataName)
g.Expect(configDataMap).ShouldNot(BeNil())
g.Expect(configDataMap.Data).Should(HaveKey("01-nova.conf"))
configData := string(configDataMap.Data["01-nova.conf"])

// Parse the INI file to properly access sections
cfg, err := ini.Load([]byte(configData))
g.Expect(err).ShouldNot(HaveOccurred(), "Should be able to parse config as INI")

// Verify region_name in [keystone_authtoken]
section := cfg.Section("keystone_authtoken")
g.Expect(section).ShouldNot(BeNil(), "Should find [keystone_authtoken] section")
g.Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [placement]
section = cfg.Section("placement")
g.Expect(section).ShouldNot(BeNil(), "Should find [placement] section")
g.Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [glance]
section = cfg.Section("glance")
g.Expect(section).ShouldNot(BeNil(), "Should find [glance] section")
g.Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [neutron]
section = cfg.Section("neutron")
g.Expect(section).ShouldNot(BeNil(), "Should find [neutron] section")
g.Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify os_region_name in [cinder]
section = cfg.Section("cinder")
g.Expect(section).ShouldNot(BeNil(), "Should find [cinder] section")
g.Expect(section.Key("os_region_name").String()).Should(Equal(testRegion))

// Verify barbican_region_name in [barbican]
section = cfg.Section("barbican")
g.Expect(section).ShouldNot(BeNil(), "Should find [barbican] section")
g.Expect(section.Key("barbican_region_name").String()).Should(Equal(testRegion))

// Verify endpoint_region_name in [oslo_limit]
section = cfg.Section("oslo_limit")
g.Expect(section).ShouldNot(BeNil(), "Should find [oslo_limit] section")
g.Expect(section.Key("endpoint_region_name").String()).Should(Equal(testRegion))
configData = string(configDataMap.Data["01-nova.conf"])
// Quick check that config contains the expected region before parsing
g.Expect(configData).To(ContainSubstring(testRegion))
}, timeout, interval).Should(Succeed())

// Parse the INI file once after Eventually succeeds (avoids repeated parsing)
cfg, err := ini.Load([]byte(configData))
Expect(err).ShouldNot(HaveOccurred(), "Should be able to parse config as INI")

// Verify region_name in [keystone_authtoken]
section := cfg.Section("keystone_authtoken")
Expect(section).ShouldNot(BeNil(), "Should find [keystone_authtoken] section")
Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [placement]
section = cfg.Section("placement")
Expect(section).ShouldNot(BeNil(), "Should find [placement] section")
Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [glance]
section = cfg.Section("glance")
Expect(section).ShouldNot(BeNil(), "Should find [glance] section")
Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [neutron]
section = cfg.Section("neutron")
Expect(section).ShouldNot(BeNil(), "Should find [neutron] section")
Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify os_region_name in [cinder]
section = cfg.Section("cinder")
Expect(section).ShouldNot(BeNil(), "Should find [cinder] section")
Expect(section.Key("os_region_name").String()).Should(Equal(testRegion))

// Verify barbican_region_name in [barbican]
section = cfg.Section("barbican")
Expect(section).ShouldNot(BeNil(), "Should find [barbican] section")
Expect(section.Key("barbican_region_name").String()).Should(Equal(testRegion))

// Verify endpoint_region_name in [oslo_limit]
section = cfg.Section("oslo_limit")
Expect(section).ShouldNot(BeNil(), "Should find [oslo_limit] section")
Expect(section.Key("endpoint_region_name").String()).Should(Equal(testRegion))
})

It("stored the input hash in the Status", func() {
Expand Down
83 changes: 43 additions & 40 deletions test/functional/novaapi_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,51 +307,54 @@ endpoint_service_type = compute`))
)

// Wait for config to be regenerated with region
var configData string
Eventually(func(g Gomega) {
configDataMap := th.GetSecret(novaNames.APIConfigDataName)
g.Expect(configDataMap).ShouldNot(BeNil())
g.Expect(configDataMap.Data).Should(HaveKey("01-nova.conf"))
configData := string(configDataMap.Data["01-nova.conf"])

// Parse the INI file to properly access sections
cfg, err := ini.Load([]byte(configData))
g.Expect(err).ShouldNot(HaveOccurred(), "Should be able to parse config as INI")

// Verify region_name in [keystone_authtoken]
section := cfg.Section("keystone_authtoken")
g.Expect(section).ShouldNot(BeNil(), "Should find [keystone_authtoken] section")
g.Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [placement]
section = cfg.Section("placement")
g.Expect(section).ShouldNot(BeNil(), "Should find [placement] section")
g.Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [glance]
section = cfg.Section("glance")
g.Expect(section).ShouldNot(BeNil(), "Should find [glance] section")
g.Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [neutron]
section = cfg.Section("neutron")
g.Expect(section).ShouldNot(BeNil(), "Should find [neutron] section")
g.Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify os_region_name in [cinder]
section = cfg.Section("cinder")
g.Expect(section).ShouldNot(BeNil(), "Should find [cinder] section")
g.Expect(section.Key("os_region_name").String()).Should(Equal(testRegion))

// Verify barbican_region_name in [barbican]
section = cfg.Section("barbican")
g.Expect(section).ShouldNot(BeNil(), "Should find [barbican] section")
g.Expect(section.Key("barbican_region_name").String()).Should(Equal(testRegion))

// Verify endpoint_region_name in [oslo_limit]
section = cfg.Section("oslo_limit")
g.Expect(section).ShouldNot(BeNil(), "Should find [oslo_limit] section")
g.Expect(section.Key("endpoint_region_name").String()).Should(Equal(testRegion))
configData = string(configDataMap.Data["01-nova.conf"])
// Quick check that config contains the expected region before parsing
g.Expect(configData).To(ContainSubstring(testRegion))
}, timeout, interval).Should(Succeed())

// Parse the INI file once after Eventually succeeds (avoids repeated parsing)
cfg, err := ini.Load([]byte(configData))
Expect(err).ShouldNot(HaveOccurred(), "Should be able to parse config as INI")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this might bite us in the futre since the nova.conf is not an ini file its in oslo config format which is a superset fo ini format but htis is a prexisign test so its fine.

just noting tha twe cant assume its ini formce as its technially not even if they default config we generate is only useing the base feature of ini


// Verify region_name in [keystone_authtoken]
section := cfg.Section("keystone_authtoken")
Expect(section).ShouldNot(BeNil(), "Should find [keystone_authtoken] section")
Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [placement]
section = cfg.Section("placement")
Expect(section).ShouldNot(BeNil(), "Should find [placement] section")
Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [glance]
section = cfg.Section("glance")
Expect(section).ShouldNot(BeNil(), "Should find [glance] section")
Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify region_name in [neutron]
section = cfg.Section("neutron")
Expect(section).ShouldNot(BeNil(), "Should find [neutron] section")
Expect(section.Key("region_name").String()).Should(Equal(testRegion))

// Verify os_region_name in [cinder]
section = cfg.Section("cinder")
Expect(section).ShouldNot(BeNil(), "Should find [cinder] section")
Expect(section.Key("os_region_name").String()).Should(Equal(testRegion))

// Verify barbican_region_name in [barbican]
section = cfg.Section("barbican")
Expect(section).ShouldNot(BeNil(), "Should find [barbican] section")
Expect(section.Key("barbican_region_name").String()).Should(Equal(testRegion))

// Verify endpoint_region_name in [oslo_limit]
section = cfg.Section("oslo_limit")
Expect(section).ShouldNot(BeNil(), "Should find [oslo_limit] section")
Expect(section.Key("endpoint_region_name").String()).Should(Equal(testRegion))
})

It("stored the input hash in the Status", func() {
Expand Down
7 changes: 7 additions & 0 deletions test/functional/novacell_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,13 @@ var _ = Describe("NovaCell controller", func() {
)

Expect(GetNovaCell(cell1.CellCRName).Status.Hash).To(HaveKey(cell1.ComputeConfigSecretName.Name))

// Verify that RabbitMQ user names are propagated to the compute-config secret
// so the openstack-operator can track which RabbitMQUser CRs are in use
Expect(computeConfigData.Data).To(
HaveKey(controllers.RabbitmqUserNameSelector))
Expect(computeConfigData.Data).To(
HaveKey(controllers.NotificationRabbitmqUserNameSelector))
})

It("should have quorum queues disabled by default and enabled when configured", func() {
Expand Down