From a6ef6c7d0418ba81ecf6e88d9bcc6e65e5ac6ca7 Mon Sep 17 00:00:00 2001 From: Prashanth Sundararaman Date: Thu, 17 Dec 2020 14:39:51 -0500 Subject: [PATCH] libvirt: support setting network dnsmasq options through the install config Since libvirt 5.6.0, there is an option to pass in dnsmasq options through the libvirt network [1]. This addresses the following problems: - eliminate the need for hacking routes in the cluster (the workaround mentioned in [3]) so that libvirt's dnsmasq does not manage the domain (and so the requests from inside the cluster will go up the chain to the host itself). - eliminate the hacky workaround used in the multi-arch CI automation to inject `*.apps` entries in the libvirt network that point to a single worker node [2]. Instead of waiting for the libvirt networks to come up and update entries, we can set this before the installation itself through the install config. - another issue this solves - with the above mentioned workaround, having multiple worker nodes becomes problematic when running upgrade tests. Having the route to just one worker node would fail the upgrade when that worker node is down. With this change, we could now point to the .1 address and have a load balancer forward traffic to any worker node. With this change, the option can be specified through the install config yaml in the network section as pairs of option name and values. An example: ``` platform: libvirt: network: dnsmasqOptions: - name: "address" value: "/.apps.tt.testing/192.168.126.51" if: tt0 ``` The terraform provider supports rendering these options through a datasource and injecting them into the network xml. Since this config is optional, not specifying it will continue to work as before without issues. [1] https://libvirt.org/formatnetwork.html#elementsNamespaces [2] https://github.com/openshift/release/blob/master/ci-operator/templates/openshift/installer/cluster-launch-installer-remote-libvirt-e2e.yaml#L532-L554 [2] https://github.com/openshift/installer/issues/1007 --- .../install.openshift.io_installconfigs.yaml | 18 ++++++ docs/dev/libvirt/README.md | 12 ++++ pkg/asset/cluster/tfvars.go | 20 +++++-- pkg/tfvars/libvirt/libvirt.go | 56 ++++++++++++------- pkg/types/libvirt/platform.go | 21 +++++++ 5 files changed, 100 insertions(+), 27 deletions(-) diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index 2e1a63981f4..4ac976735ec 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -1479,6 +1479,24 @@ spec: network: description: Network properties: + dnsmasqOptions: + description: DnsmasqOptions is the dnsmasq options to be used + when installing on libvirt. + items: + description: DnsmasqOption contains the name and value of + the option to configure in the libvirt network. + properties: + name: + description: The dnsmasq option name. A full list of + options and an explanation for each can be found in + /etc/dnsmasq.conf + type: string + value: + description: The value that is being set for the particular + option. + type: string + type: object + type: array if: default: tt0 description: The interface make used for the network. Default diff --git a/docs/dev/libvirt/README.md b/docs/dev/libvirt/README.md index bb0bdfe212e..8ffc3432362 100644 --- a/docs/dev/libvirt/README.md +++ b/docs/dev/libvirt/README.md @@ -345,6 +345,18 @@ server=/tt.testing/192.168.126.1 address=/.apps.tt.testing/192.168.126.51 ``` +- An alternate method to specify the dnsmasq option, if the system is using libvirt version 5.6.0+, is to specify the option in the install config under the platform's network section in the following way. + +``` + platform: + libvirt: + network: + dnsmasqOptions: + - name: "address" + value: "/.apps.tt.testing/192.168.126.51" + if: tt0 +``` + - Make sure you restart the NetworkManager after change in `openshift.conf`: ```console diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index db3ae498535..2d9d020f727 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -366,13 +366,21 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { if err != nil { return err } + // convert options list to a map which can be consumed by terraform + dnsmasqoptions := make(map[string]string) + for _, option := range installConfig.Config.Platform.Libvirt.Network.DnsmasqOptions { + dnsmasqoptions[option.Name] = option.Value + } data, err = libvirttfvars.TFVars( - masters[0].Spec.ProviderSpec.Value.Object.(*libvirtprovider.LibvirtMachineProviderConfig), - string(*rhcosImage), - &installConfig.Config.Networking.MachineNetwork[0].CIDR.IPNet, - installConfig.Config.Platform.Libvirt.Network.IfName, - masterCount, - installConfig.Config.ControlPlane.Architecture, + libvirttfvars.TFVarsSources{ + MasterConfig: masters[0].Spec.ProviderSpec.Value.Object.(*libvirtprovider.LibvirtMachineProviderConfig), + OsImage: string(*rhcosImage), + MachineCIDR: &installConfig.Config.Networking.MachineNetwork[0].CIDR.IPNet, + Bridge: installConfig.Config.Platform.Libvirt.Network.IfName, + MasterCount: masterCount, + Architecture: installConfig.Config.ControlPlane.Architecture, + DnsmasqOptions: dnsmasqoptions, + }, ) if err != nil { return errors.Wrapf(err, "failed to get %s Terraform variables", platform) diff --git a/pkg/tfvars/libvirt/libvirt.go b/pkg/tfvars/libvirt/libvirt.go index dfdfd8a3a95..e669ffe8987 100644 --- a/pkg/tfvars/libvirt/libvirt.go +++ b/pkg/tfvars/libvirt/libvirt.go @@ -18,29 +18,42 @@ import ( ) type config struct { - URI string `json:"libvirt_uri,omitempty"` - Image string `json:"os_image,omitempty"` - IfName string `json:"libvirt_network_if"` - MasterIPs []string `json:"libvirt_master_ips,omitempty"` - BootstrapIP string `json:"libvirt_bootstrap_ip,omitempty"` - MasterMemory string `json:"libvirt_master_memory,omitempty"` - MasterVcpu string `json:"libvirt_master_vcpu,omitempty"` - BootstrapMemory int `json:"libvirt_bootstrap_memory,omitempty"` - MasterDiskSize string `json:"libvirt_master_size,omitempty"` + URI string `json:"libvirt_uri,omitempty"` + Image string `json:"os_image,omitempty"` + IfName string `json:"libvirt_network_if"` + MasterIPs []string `json:"libvirt_master_ips,omitempty"` + BootstrapIP string `json:"libvirt_bootstrap_ip,omitempty"` + MasterMemory string `json:"libvirt_master_memory,omitempty"` + MasterVcpu string `json:"libvirt_master_vcpu,omitempty"` + BootstrapMemory int `json:"libvirt_bootstrap_memory,omitempty"` + MasterDiskSize string `json:"libvirt_master_size,omitempty"` + DnsmasqOptions map[string]string `json:"libvirt_dnsmasq_options,omitempty"` +} + +// TFVarsSources contains the parameters to be converted into Terraform variables +type TFVarsSources struct { + MasterConfig *v1beta1.LibvirtMachineProviderConfig + OsImage string + MachineCIDR *net.IPNet + Bridge string + MasterCount int + Architecture types.Architecture + DnsmasqOptions map[string]string } // TFVars generates libvirt-specific Terraform variables. -func TFVars(masterConfig *v1beta1.LibvirtMachineProviderConfig, osImage string, machineCIDR *net.IPNet, bridge string, masterCount int, architecture types.Architecture) ([]byte, error) { - bootstrapIP, err := cidr.Host(machineCIDR, 10) +func TFVars(sources TFVarsSources) ([]byte, error) { + bootstrapIP, err := cidr.Host(sources.MachineCIDR, 10) if err != nil { return nil, errors.Errorf("failed to generate bootstrap IP: %v", err) } - masterIPs, err := generateIPs("master", machineCIDR, masterCount, 11) + masterIPs, err := generateIPs("master", sources.MachineCIDR, sources.MasterCount, 11) if err != nil { return nil, err } + osImage := sources.OsImage url, err := url.Parse(osImage) if err != nil { return nil, errors.Wrap(err, "failed to parse image url") @@ -59,17 +72,18 @@ func TFVars(masterConfig *v1beta1.LibvirtMachineProviderConfig, osImage string, } cfg := &config{ - URI: masterConfig.URI, - Image: osImage, - IfName: bridge, - BootstrapIP: bootstrapIP.String(), - MasterIPs: masterIPs, - MasterMemory: strconv.Itoa(masterConfig.DomainMemory), - MasterVcpu: strconv.Itoa(masterConfig.DomainVcpu), + URI: sources.MasterConfig.URI, + Image: osImage, + IfName: sources.Bridge, + BootstrapIP: bootstrapIP.String(), + MasterIPs: masterIPs, + MasterMemory: strconv.Itoa(sources.MasterConfig.DomainMemory), + MasterVcpu: strconv.Itoa(sources.MasterConfig.DomainVcpu), + DnsmasqOptions: sources.DnsmasqOptions, } - if masterConfig.Volume.VolumeSize != nil { - cfg.MasterDiskSize = masterConfig.Volume.VolumeSize.String() + if sources.MasterConfig.Volume.VolumeSize != nil { + cfg.MasterDiskSize = sources.MasterConfig.Volume.VolumeSize.String() } return json.MarshalIndent(cfg, "", " ") diff --git a/pkg/types/libvirt/platform.go b/pkg/types/libvirt/platform.go index 8f532fc609a..ffe6d7a6116 100644 --- a/pkg/types/libvirt/platform.go +++ b/pkg/types/libvirt/platform.go @@ -33,4 +33,25 @@ type Network struct { // +kubebuilder:default="tt0" // +optional IfName string `json:"if,omitempty"` + + // DnsmasqOptions is the dnsmasq options to be used when installing on + // libvirt. + // + // +optional + DnsmasqOptions []DnsmasqOption `json:"dnsmasqOptions,omitempty"` +} + +// DnsmasqOption contains the name and value of the option to configure in the +// libvirt network. +type DnsmasqOption struct { + // The dnsmasq option name. A full list of options and an explanation for + // each can be found in /etc/dnsmasq.conf + // + // +optional + Name string `json:"name,omitempty"` + + // The value that is being set for the particular option. + // + // +optional + Value string `json:"value,omitempty"` }