[VNC]Cleanup configuration

While reviewing other PRs we noticed that there is some unused template
parameters for the vnc configurations. So I started cleaning up it. Then
I noticed that never filled template parameters in our nova.conf related
to vnc. So this PR cleans up the [vnc] section and the related config
generation code. To be able to follow our template I split the template
to compute and vncproxy parts and made sure that only the relevant
configs are applied for each service. Also fixed a small bug that
ignored config generation errors.

controllers/novacell_controller.go

@@ -600,6 +600,10 @@ func (r *NovaCellReconciler) generateComputeConfigs(
 	err := r.GenerateConfigs(
 		ctx, h, instance, configName, &hashes, templateParameters, map[string]string{}, cmLabels, map[string]string{},
 	)
+	if err != nil {
+		return err
+	}
+
 	// TODO(gibi): can we make it simpler?
 	a := &corev1.EnvVar{}
 	hashes[configName](a)

controllers/novanovncproxy_controller.go

@@ -285,8 +285,6 @@ func (r *NovaNoVNCProxyReconciler) generateConfigs(
 		"cell_db_password":       string(secret.Data[CellDatabasePasswordSelector]),
 		"cell_db_address":        instance.Spec.CellDatabaseHostname,
 		"cell_db_port":           3306,
-		"api_interface_address":  "",     // fixme
-		"public_protocol":        "http", // fixme
 		"transport_url":          string(secret.Data[TransportURLSelector]),
 		"openstack_cacert":       "",          // fixme
 		"openstack_region_name":  "regionOne", // fixme

templates/nova.conf

@@ -117,23 +117,29 @@ driver = noop
 notify_on_state_change = vm_and_task_state
 {{ end }}
 
-{{ if eq .service_name "nova-compute" "nova-novncproxy"}}
+{{ if eq .service_name "nova-novncproxy"}}
 [vnc]
 enabled = True
-novncproxy_host = {{ if (index . "novncproxy_service_host") }} {{ .novncproxy_service_host }} {{else}}"::0"{{ end }}
-novncproxy_port = {{ if (index . "nova_novncproxy_listen_port") }}{{ .nova_novncproxy_listen_port }} {{else}}6080{{ end }}
-server_listen = {{ if (index . "novncproxy_service_host") }}{{ .novncproxy_service_host }} {{else}}"::0"{{ end }}
+novncproxy_host = "::0"
+novncproxy_port = 6080
+{{end}}
+
 {{ if eq .service_name "nova-compute"}}
+{{ if (index . "novncproxy_base_url") }}
+[vnc]
+enabled = True
+novncproxy_base_url = {{ .novncproxy_base_url }}
+server_listen = "::0"
 {{/* https://docs.openstack.org/oslo.config/latest/configuration/format.html#substitution    */}}
 # note we may want to use console_host instead of my_ip however it wont be resolved via
 # dns currently so we need to use my_ip for now.
 # https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.console_host
 server_proxyclient_address = "$my_ip"
-{{if (index . "novncproxy_base_url")}}
-novncproxy_base_url = {{ .novncproxy_base_url }}
-{{ end }}
-{{ end }}
-{{ end }}
+{{else}}
+[vnc]
+enabled = False
+{{end}}
+{{end}}
 
 [cache]
 # always enable caching

test/functional/nova_novncproxy_test.go

@@ -162,7 +162,6 @@ var _ = Describe("NovaNoVNCProxy controller", func() {
 				configData := string(configDataMap.Data["01-nova.conf"])
 				Expect(configData).Should(ContainSubstring("novncproxy_host = \"::0\""))
 				Expect(configData).Should(ContainSubstring("novncproxy_port = 6080"))
-				Expect(configData).Should(ContainSubstring("server_listen = \"::0\""))
 				Expect(configData).Should(ContainSubstring("password = service-password"))
 				Expect(configData).Should(ContainSubstring("transport_url=rabbit://cell1/fake"))
 				Expect(configDataMap.Data).Should(HaveKey("02-nova-override.conf"))

test/functional/novacell_controller_test.go

@@ -266,6 +266,7 @@ var _ = Describe("NovaCell controller", func() {
 			vncUrlConfig := fmt.Sprintf("novncproxy_base_url = http://%s/vnc_lite.html",
 				fmt.Sprintf("nova-novncproxy-%s-public.%s.svc:6080", cell1.CellName, cell1.CellCRName.Namespace))
 			Expect(configData).To(ContainSubstring(vncUrlConfig))
+			Expect(configData).To(ContainSubstring("[vnc]\nenabled = True"))
 
 			th.ExpectCondition(
 				cell1.CellCRName,
@@ -312,8 +313,9 @@ var _ = Describe("NovaCell controller", func() {
 			configData := string(computeConfigData.Data["01-nova.conf"])
 			Expect(configData).To(ContainSubstring("transport_url=rabbit://cell1/fake"))
 			Expect(configData).To(ContainSubstring("username = nova\npassword = service-password\n"))
-			vncUrlConfig := fmt.Sprintf("novncproxy_base_url = http://foo/vnc_lite.html")
+			vncUrlConfig := "novncproxy_base_url = http://foo/vnc_lite.html"
 			Expect(configData).To(ContainSubstring(vncUrlConfig))
+			Expect(configData).To(ContainSubstring("[vnc]\nenabled = True"))
 
 			th.ExpectCondition(
 				cell1.CellCRName,
@@ -369,6 +371,14 @@ var _ = Describe("NovaCell controller", func() {
 				corev1.ConditionTrue,
 			)
 
+			// updates the compute config and disables VNC there
+			computeConfigData := th.GetSecret(cell1.ComputeConfigSecretName)
+			Expect(computeConfigData).ShouldNot(BeNil())
+			Expect(computeConfigData.Data).Should(HaveKey("01-nova.conf"))
+			configData := string(computeConfigData.Data["01-nova.conf"])
+			Expect(configData).NotTo(ContainSubstring("novncproxy_base_url"))
+			Expect(configData).To(ContainSubstring("[vnc]\nenabled = False"))
+
 		})
 		It("deletes NovaMetadata if it is disabled later", func() {
 			th.SimulateJobSuccess(cell1.DBSyncJobName)
@@ -429,8 +439,9 @@ var _ = Describe("NovaCell controller", func() {
 			Expect(configData).To(ContainSubstring("transport_url=rabbit://cell2/fake"))
 			Expect(configData).To(ContainSubstring("username = nova\npassword = service-password\n"))
 			// There is no VNCProxy created but we still get a compute config just
-			// without any vnc proxy url
-			Expect(configData).NotTo(ContainSubstring("novncproxy_base_url "))
+			// without any vnc proxy url and therefore vnc disabled
+			Expect(configData).NotTo(ContainSubstring("novncproxy_base_url"))
+			Expect(configData).To(ContainSubstring("[vnc]\nenabled = False"))
 
 			th.ExpectCondition(
 				cell2.CellCRName,
@@ -514,6 +525,7 @@ var _ = Describe("NovaCell controller", func() {
 			vncUrlConfig := fmt.Sprintf("novncproxy_base_url = http://%s/vnc_lite.html",
 				fmt.Sprintf("nova-novncproxy-%s-public.%s.svc:6080", cell2.CellName, cell2.CellCRName.Namespace))
 			Expect(configData).To(ContainSubstring(vncUrlConfig))
+			Expect(configData).To(ContainSubstring("[vnc]\nenabled = True"))
 
 			Expect(GetNovaCell(cell2.CellCRName).Status.Hash[cell2.ComputeConfigSecretName.Name]).NotTo(BeNil())
 			Expect(GetNovaCell(cell2.CellCRName).Status.Hash[cell2.ComputeConfigSecretName.Name]).NotTo(Equal(oldComputeConfigHash))