Repository: openshift/assisted-service (language: python)

Issue:
 [release-ocm-2.10] MGMT-17187: Disable ironic vlan creation when static networking config is present
This is an automated cherry-pick of #6055

/assign gamli75


Discussion / hints:

Fix patch:
diff --git a/go.mod b/go.mod
index 3406ed366bb..b1aacec9173 100644
--- a/go.mod
+++ b/go.mod
@@ -56,7 +56,7 @@ require (
 	github.com/openshift/custom-resource-status v1.1.2
 	github.com/openshift/generic-admission-server v1.14.1-0.20231020105858-8dcc3c9b298f
 	github.com/openshift/hive/apis v0.0.0-20220222213051-def9088fdb5a
-	github.com/openshift/image-customization-controller v0.0.0-20231018032321-4677f060c4bf
+	github.com/openshift/image-customization-controller v0.0.0-20240129110832-60a3867d7f9e
 	github.com/ory/dockertest/v3 v3.9.1
 	github.com/patrickmn/go-cache v2.1.0+incompatible
 	github.com/pelletier/go-toml v1.9.5
diff --git a/go.sum b/go.sum
index 9fecf4a0f8a..9b59f120453 100644
--- a/go.sum
+++ b/go.sum
@@ -1371,8 +1371,8 @@ github.com/openshift/generic-admission-server v1.14.1-0.20231020105858-8dcc3c9b2
 github.com/openshift/generic-admission-server v1.14.1-0.20231020105858-8dcc3c9b298f/go.mod h1:/CLsleDcQ6AFTGKJe9VL3Y4rB9DqX3fQwQv47q2/ZJc=
 github.com/openshift/hive/apis v0.0.0-20220222213051-def9088fdb5a h1:E+XPJs/aVvYsrlJzo2ED38ZTR2RTNUlFMmOaFAAdMZg=
 github.com/openshift/hive/apis v0.0.0-20220222213051-def9088fdb5a/go.mod h1:E1bgquRiwfugdArdecPbpYIrAdve5kTzMaJb0+8jMXI=
-github.com/openshift/image-customization-controller v0.0.0-20231018032321-4677f060c4bf h1:Z+LlKXsPGtzMsIQxSWAtebZLlUV/o678wYReGItPSkc=
-github.com/openshift/image-customization-controller v0.0.0-20231018032321-4677f060c4bf/go.mod h1:rTvO75VGcFhEuE2HgQe10OijcR9HBDNC+CnKei8p1HE=
+github.com/openshift/image-customization-controller v0.0.0-20240129110832-60a3867d7f9e h1:x1j5gWm4PdTTXTmuX3S7VAmsnazbbMiefQyCLdyjH74=
+github.com/openshift/image-customization-controller v0.0.0-20240129110832-60a3867d7f9e/go.mod h1:rTvO75VGcFhEuE2HgQe10OijcR9HBDNC+CnKei8p1HE=
 github.com/openshift/library-go v0.0.0-20191003152030-97c62d8a2901/go.mod h1:NBttNjZpWwup/nthuLbPAPSYC8Qyo+BBK5bCtFoyYjo=
 github.com/openshift/library-go v0.0.0-20200831114015-2ab0c61c15de/go.mod h1:6vwp+YhYOIlj8MpkQKkebTTSn2TuYyvgiAFQ206jIEQ=
 github.com/openshift/library-go v0.0.0-20231110170715-08d73a9c798b h1:BmunS/b02dKSwQ0H21+3EbXDfDr9t8UDlO54WxKkOts=
diff --git a/internal/ignition/ironic.go b/internal/ignition/ironic.go
index 6c48fb9cf19..eb3f97cac59 100644
--- a/internal/ignition/ironic.go
+++ b/internal/ignition/ironic.go
@@ -13,12 +13,17 @@ func GenerateIronicConfig(ironicBaseURL string, ironicInspectorURL string, infra
 	config.Ignition.Version = "3.2.0"
 
 	httpProxy, httpsProxy, noProxy := common.GetProxyConfigs(infraEnv.Proxy)
+	ironicInspectorVlanInterfaces := ""
+	// allow ironic to configure vlans if no static network config is provided
+	if infraEnv.StaticNetworkConfig == "" {
+		ironicInspectorVlanInterfaces = "all"
+	}
 	// TODO: this should probably get the pullSecret as well
-	ib, err := iccignition.New([]byte{}, []byte{}, ironicBaseURL, ironicInspectorURL, ironicAgentImage, "", "", "", httpProxy, httpsProxy, noProxy, "")
+	ib, err := iccignition.New([]byte{}, []byte{}, ironicBaseURL, ironicInspectorURL, ironicAgentImage, "", "", "", httpProxy, httpsProxy, noProxy, "", ironicInspectorVlanInterfaces)
 	if err != nil {
 		return []byte{}, err
 	}
-	config.Storage.Files = []ignition_config_types_32.File{ib.IronicAgentConf()}
+	config.Storage.Files = []ignition_config_types_32.File{ib.IronicAgentConf(ironicInspectorVlanInterfaces)}
 	// TODO: sort out the flags (authfile...) and copy network
 	config.Systemd.Units = []ignition_config_types_32.Unit{ib.IronicAgentService(false)}
 	return json.Marshal(config)
diff --git a/vendor/github.com/openshift/image-customization-controller/pkg/ignition/builder.go b/vendor/github.com/openshift/image-customization-controller/pkg/ignition/builder.go
index f4bcf100c3c..8d69471b857 100644
--- a/vendor/github.com/openshift/image-customization-controller/pkg/ignition/builder.go
+++ b/vendor/github.com/openshift/image-customization-controller/pkg/ignition/builder.go
@@ -14,28 +14,26 @@ import (
 const (
 	// https://github.com/openshift/ironic-image/blob/master/scripts/configure-coreos-ipa#L14
 	ironicAgentPodmanFlags = "--tls-verify=false"
-
-	// https://github.com/openshift/ironic-image/blob/master/scripts/configure-coreos-ipa#L11
-	ironicInspectorVlanInterfaces = "all"
 )
 
 type ignitionBuilder struct {
-	nmStateData            []byte
-	registriesConf         []byte
-	ironicBaseURL          string
-	ironicInspectorBaseURL string
-	ironicAgentImage       string
-	ironicAgentPullSecret  string
-	ironicRAMDiskSSHKey    string
-	networkKeyFiles        []byte
-	ipOptions              string
-	httpProxy              string
-	httpsProxy             string
-	noProxy                string
-	hostname               string
+	nmStateData               []byte
+	registriesConf            []byte
+	ironicBaseURL             string
+	ironicInspectorBaseURL    string
+	ironicAgentImage          string
+	ironicAgentPullSecret     string
+	ironicRAMDiskSSHKey       string
+	networkKeyFiles           []byte
+	ipOptions                 string
+	httpProxy                 string
+	httpsProxy                string
+	noProxy                   string
+	hostname                  string
+	ironicAgentVlanInterfaces string
 }
 
-func New(nmStateData, registriesConf []byte, ironicBaseURL, ironicInspectorBaseURL, ironicAgentImage, ironicAgentPullSecret, ironicRAMDiskSSHKey, ipOptions string, httpProxy, httpsProxy, noProxy string, hostname string) (*ignitionBuilder, error) {
+func New(nmStateData, registriesConf []byte, ironicBaseURL, ironicInspectorBaseURL, ironicAgentImage, ironicAgentPullSecret, ironicRAMDiskSSHKey, ipOptions string, httpProxy, httpsProxy, noProxy string, hostname string, ironicAgentVlanInterfaces string) (*ignitionBuilder, error) {
 	if ironicBaseURL == "" {
 		return nil, errors.New("ironicBaseURL is required")
 	}
@@ -47,18 +45,19 @@ func New(nmStateData, registriesConf []byte, ironicBaseURL, ironicInspectorBaseU
 	}
 
 	return &ignitionBuilder{
-		nmStateData:            nmStateData,
-		registriesConf:         registriesConf,
-		ironicBaseURL:          ironicBaseURL,
-		ironicInspectorBaseURL: ironicInspectorBaseURL,
-		ironicAgentImage:       ironicAgentImage,
-		ironicAgentPullSecret:  ironicAgentPullSecret,
-		ironicRAMDiskSSHKey:    ironicRAMDiskSSHKey,
-		ipOptions:              ipOptions,
-		httpProxy:              httpProxy,
-		httpsProxy:             httpsProxy,
-		noProxy:                noProxy,
-		hostname:               hostname,
+		nmStateData:               nmStateData,
+		registriesConf:            registriesConf,
+		ironicBaseURL:             ironicBaseURL,
+		ironicInspectorBaseURL:    ironicInspectorBaseURL,
+		ironicAgentImage:          ironicAgentImage,
+		ironicAgentPullSecret:     ironicAgentPullSecret,
+		ironicRAMDiskSSHKey:       ironicRAMDiskSSHKey,
+		ipOptions:                 ipOptions,
+		httpProxy:                 httpProxy,
+		httpsProxy:                httpsProxy,
+		noProxy:                   noProxy,
+		hostname:                  hostname,
+		ironicAgentVlanInterfaces: ironicAgentVlanInterfaces,
 	}, nil
 }
 
@@ -97,8 +96,21 @@ func (b *ignitionBuilder) GenerateConfig() (config ignition_config_types_32.Conf
 		}
 	}
 
+	var ironicInspectorVlanInterfaces string
+	if strings.ToLower(b.ironicAgentVlanInterfaces) == "always" {
+		ironicInspectorVlanInterfaces = "all"
+	} else if strings.ToLower(b.ironicAgentVlanInterfaces) == "never" {
+		ironicInspectorVlanInterfaces = ""
+	} else {
+		if len(b.nmStateData) > 0 {
+			ironicInspectorVlanInterfaces = ""
+		} else {
+			ironicInspectorVlanInterfaces = "all"
+		}
+	}
+
 	config.Ignition.Version = "3.2.0"
-	config.Storage.Files = []ignition_config_types_32.File{b.IronicAgentConf()}
+	config.Storage.Files = []ignition_config_types_32.File{b.IronicAgentConf(ironicInspectorVlanInterfaces)}
 	config.Storage.Files = append(config.Storage.Files, netFiles...)
 	config.Systemd.Units = []ignition_config_types_32.Unit{b.IronicAgentService(len(netFiles) > 0)}
 
diff --git a/vendor/github.com/openshift/image-customization-controller/pkg/ignition/service_config.go b/vendor/github.com/openshift/image-customization-controller/pkg/ignition/service_config.go
index 4cdb738dc52..5e21070571c 100644
--- a/vendor/github.com/openshift/image-customization-controller/pkg/ignition/service_config.go
+++ b/vendor/github.com/openshift/image-customization-controller/pkg/ignition/service_config.go
@@ -2,21 +2,57 @@ package ignition
 
 import (
 	"fmt"
+	"net"
+	"net/url"
 	"strings"
 
 	ignition_config_types_32 "github.com/coreos/ignition/v2/config/v3_2/types"
 	"k8s.io/utils/pointer"
 )
 
-func (b *ignitionBuilder) IronicAgentConf() ignition_config_types_32.File {
+const (
+	defaultIronicPort    = "6385"
+	defaultInspectorPort = "5050"
+)
+
+func processURLs(baseURL, defaultPath, defaultPort string) string {
+	urls := strings.Split(baseURL, ",")
+	var result []string
+	for _, urlString := range urls {
+		if urlString == "" {
+			continue // tolerate empty strings or trailing commas
+		}
+
+		parsed, err := url.Parse(urlString)
+		if err != nil {
+			continue // I wish we had a logger here...
+		}
+
+		if defaultPort != "" && parsed.Port() == "" {
+			parsed.Host = net.JoinHostPort(parsed.Hostname(), defaultPort)
+		}
+
+		if defaultPath != "" && !strings.HasSuffix(parsed.Path, defaultPath) {
+			parsed.Path = fmt.Sprintf("%s%s", parsed.Path, defaultPath)
+		}
+
+		result = append(result, parsed.String())
+	}
+
+	return strings.Join(result, ",")
+}
+
+func (b *ignitionBuilder) IronicAgentConf(ironicInspectorVlanInterfaces string) ignition_config_types_32.File {
 	template := `
 [DEFAULT]
-api_url = %s:6385
-inspection_callback_url = %s:5050/v1/continue
+api_url = %s
+inspection_callback_url = %s
 insecure = True
 enable_vlan_interfaces = %s
 `
-	contents := fmt.Sprintf(template, b.ironicBaseURL, b.ironicInspectorBaseURL, ironicInspectorVlanInterfaces)
+	ironicURLs := processURLs(b.ironicBaseURL, "", defaultIronicPort)
+	inspectorURLs := processURLs(b.ironicInspectorBaseURL, "/v1/continue", defaultInspectorPort)
+	contents := fmt.Sprintf(template, ironicURLs, inspectorURLs, ironicInspectorVlanInterfaces)
 	return ignitionFileEmbed("/etc/ironic-python-agent.conf", 0644, false, []byte(contents))
 }
 
diff --git a/vendor/modules.txt b/vendor/modules.txt
index b7ffdb10188..5ce96814460 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -774,7 +774,7 @@ github.com/openshift/hive/apis/hive/v1/openstack
 github.com/openshift/hive/apis/hive/v1/ovirt
 github.com/openshift/hive/apis/hive/v1/vsphere
 github.com/openshift/hive/apis/scheme
-# github.com/openshift/image-customization-controller v0.0.0-20231018032321-4677f060c4bf
+# github.com/openshift/image-customization-controller v0.0.0-20240129110832-60a3867d7f9e
 ## explicit; go 1.19
 github.com/openshift/image-customization-controller/pkg/ignition
 # github.com/openshift/library-go v0.0.0-20231110170715-08d73a9c798b


Test patch:
diff --git a/internal/ignition/ironic_test.go b/internal/ignition/ironic_test.go
index b0c40d049ae..a4149156ded 100644
--- a/internal/ignition/ironic_test.go
+++ b/internal/ignition/ironic_test.go
@@ -9,6 +9,7 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/openshift/assisted-service/internal/common"
 	"github.com/openshift/assisted-service/models"
+	"github.com/vincent-petithory/dataurl"
 )
 
 var _ = Describe("GenerateIronicConfig", func() {
@@ -45,6 +46,17 @@ var _ = Describe("GenerateIronicConfig", func() {
 		_, err := GenerateIronicConfig(ironicBaseURL, inspectorURL, infraEnv, "")
 		Expect(err).To(HaveOccurred())
 	})
+	It("set the ironic inspector config to not tag interfaces when static networking is configured", func() {
+		infraEnv.StaticNetworkConfig = "some network config here"
+		conf, err := GenerateIronicConfig(ironicBaseURL, inspectorURL, infraEnv, "ironicAgentImage:custom")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(conf).Should(ContainSubstring(dataurl.Escape([]byte("enable_vlan_interfaces = \n"))))
+	})
+	It("sets the ironic inspector config to tag all interfaces when static networking is not configured", func() {
+		conf, err := GenerateIronicConfig(ironicBaseURL, inspectorURL, infraEnv, "ironicAgentImage:custom")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(conf).Should(ContainSubstring(dataurl.Escape([]byte("enable_vlan_interfaces = all\n"))))
+	})
 })
 
 func validateIgnition(conf []byte) {