Skip to content

Commit

Permalink
MGMT-16494: Move ip hint file creation to ignition in order to change…
Browse files Browse the repository at this point in the history 
… it in IBI process
  • Loading branch information
@tsorya
tsorya committed Jan 3, 2024
1 parent 315899d commit 9cd73e3
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 195 deletions.
4 changes: 0 additions & 4 deletions internal/cluster/cluster.go
View file
Expand Up @@ -1644,10 +1644,6 @@ func (m *Manager) GenerateAdditionalManifests(ctx context.Context, cluster *comm
return errors.Wrap(err, "failed to add disk encryption manifest")
}

if err := m.manifestsGeneratorAPI.AddNodeIpHint(ctx, log, cluster); err != nil {
return errors.Wrap(err, "failed to add node ip hint")
}

return nil
}

Expand Down
3 changes: 0 additions & 3 deletions internal/cluster/cluster_test.go
View file
Expand Up @@ -2956,7 +2956,6 @@ var _ = Describe("GenerateAdditionalManifests", func() {
manifestsGenerator.EXPECT().AddChronyManifest(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)
manifestsGenerator.EXPECT().IsSNODNSMasqEnabled().Return(true).Times(1)
manifestsGenerator.EXPECT().AddDnsmasqForSingleNode(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)
manifestsGenerator.EXPECT().AddNodeIpHint(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)
manifestsGenerator.EXPECT().AddTelemeterManifest(ctx, gomock.Any(), &c).Return(nil)
manifestsGenerator.EXPECT().AddDiskEncryptionManifest(ctx, gomock.Any(), &c).Return(nil)
mockOperatorMgr.EXPECT().GenerateManifests(gomock.Any(), gomock.Any()).Return(nil).Times(1)
Expand All @@ -2979,7 +2978,6 @@ var _ = Describe("GenerateAdditionalManifests", func() {
capi = NewManager(cfg2, common.GetTestLog(), db, commontesting.GetDummyNotificationStream(ctrl), eventsHandler, nil, nil, mockMetric, manifestsGenerator, nil, mockOperatorMgr, nil, nil, nil, nil, nil)
manifestsGenerator.EXPECT().AddChronyManifest(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)
manifestsGenerator.EXPECT().IsSNODNSMasqEnabled().Return(false).Times(1)
manifestsGenerator.EXPECT().AddNodeIpHint(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)
manifestsGenerator.EXPECT().AddTelemeterManifest(ctx, gomock.Any(), &c).Return(nil)
manifestsGenerator.EXPECT().AddDiskEncryptionManifest(ctx, gomock.Any(), &c).Return(nil)
mockOperatorMgr.EXPECT().GenerateManifests(gomock.Any(), gomock.Any()).Return(nil).Times(1)
Expand All @@ -3005,7 +3003,6 @@ var _ = Describe("GenerateAdditionalManifests", func() {
manifestsGenerator.EXPECT().AddChronyManifest(ctx, gomock.Any(), &c).Return(nil)
mockOperatorMgr.EXPECT().GenerateManifests(ctx, &c).Return(nil)
manifestsGenerator.EXPECT().AddTelemeterManifest(ctx, gomock.Any(), &c).Return(nil)
manifestsGenerator.EXPECT().AddNodeIpHint(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)
manifestsGenerator.EXPECT().AddDiskEncryptionManifest(ctx, gomock.Any(), &c).Return(nil)

err := capi.GenerateAdditionalManifests(ctx, &c)
Expand Down
27 changes: 18 additions & 9 deletions internal/ignition/ignition.go
View file
Expand Up @@ -58,8 +58,9 @@ import (
)

const (
masterIgn = "master.ign"
workerIgn = "worker.ign"
masterIgn = "master.ign"
workerIgn = "worker.ign"
nodeIpHintFile = "/etc/default/nodeip-configuration"
)

// Names of some relevant templates:
Expand Down Expand Up @@ -451,7 +452,7 @@ func (g *installerGenerator) Generate(ctx context.Context, installConfig []byte,
return err
}

err = g.createHostIgnitions(g.serviceBaseURL, authType)
err = g.createHostIgnitions()
if err != nil {
log.Error(err)
return err
Expand Down Expand Up @@ -1510,7 +1511,7 @@ func (g *installerGenerator) modifyPointerIgnitionMCP(poolName string, ignitionS
return "", errors.Errorf("machine config pool %s was not found", poolName)
}

func (g *installerGenerator) writeSingleHostFile(host *models.Host, baseFile string, workDir, serviceBaseURL, bootReporter string, authType auth.AuthType) error {
func (g *installerGenerator) writeSingleHostFile(host *models.Host, baseFile string, workDir string) error {
config, err := parseIgnitionFile(filepath.Join(workDir, baseFile))
if err != nil {
return err
Expand All @@ -1522,6 +1523,14 @@ func (g *installerGenerator) writeSingleHostFile(host *models.Host, baseFile str
}

setFileInIgnition(config, "/etc/hostname", fmt.Sprintf("data:,%s", hostname), false, 420, true)
if common.IsSingleNodeCluster(g.cluster) {
machineCidr := g.cluster.MachineNetworks[0]
ip, _, errP := net.ParseCIDR(string(machineCidr.Cidr))
if errP != nil {
return errors.Wrapf(errP, "Failed to parse machine cidr for node ip hint content")
}
setFileInIgnition(config, nodeIpHintFile, fmt.Sprintf("data:,KUBELET_NODEIP_HINT=%s", ip), false, 420, true)
}

configBytes, err := json.Marshal(config)
if err != nil {
Expand Down Expand Up @@ -1554,28 +1563,28 @@ func (g *installerGenerator) writeSingleHostFile(host *models.Host, baseFile str
return nil
}

func (g *installerGenerator) writeHostFiles(hosts []*models.Host, baseFile string, workDir string, serviceBaseURL string, authType auth.AuthType) error {
func (g *installerGenerator) writeHostFiles(hosts []*models.Host, baseFile string, workDir string) error {
errGroup := new(errgroup.Group)
for i := range hosts {
host := hosts[i]
errGroup.Go(func() error {
return g.writeSingleHostFile(host, baseFile, workDir, serviceBaseURL, "", authType)
return g.writeSingleHostFile(host, baseFile, workDir)
})
}

return errGroup.Wait()
}

// createHostIgnitions builds an ignition file for each host in the cluster based on the generated <role>.ign file
func (g *installerGenerator) createHostIgnitions(serviceBaseURL string, authType auth.AuthType) error {
func (g *installerGenerator) createHostIgnitions() error {
masters, workers := sortHosts(g.cluster.Hosts)

err := g.writeHostFiles(masters, masterIgn, g.workDir, serviceBaseURL, authType)
err := g.writeHostFiles(masters, masterIgn, g.workDir)
if err != nil {
return errors.Wrapf(err, "error writing master host ignition files")
}

err = g.writeHostFiles(workers, workerIgn, g.workDir, serviceBaseURL, authType)
err = g.writeHostFiles(workers, workerIgn, g.workDir)
if err != nil {
return errors.Wrapf(err, "error writing worker host ignition files")
}
Expand Down
96 changes: 92 additions & 4 deletions internal/ignition/ignition_test.go
View file
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/openshift/assisted-service/internal/network"
"io"
"io/fs"
"os"
Expand Down Expand Up @@ -546,7 +547,7 @@ var _ = Describe("createHostIgnitions", func() {
g := NewGenerator("", workDir, installerCacheDir, cluster, "", "", "", "", nil, log,
mockOperatorManager, mockProviderRegistry, "", "", 5).(*installerGenerator)

err := g.createHostIgnitions("http://www.example.com:6008", auth.TypeRHSSO)
err := g.createHostIgnitions()
Expect(err).NotTo(HaveOccurred())

for _, host := range cluster.Hosts {
Expand Down Expand Up @@ -581,6 +582,93 @@ var _ = Describe("createHostIgnitions", func() {
})
})

Context("node ip hint", func() {
It("SNO: adds nodeip hint file", func() {
cluster.Hosts = []*models.Host{
{
RequestedHostname: "master0.example.com",
Role: models.HostRoleMaster,
},
}
cluster.HighAvailabilityMode = swag.String(models.ClusterHighAvailabilityModeNone)
cluster.MachineNetworks = network.CreateMachineNetworksArray("3.3.3.0/24")

// create an ID for each host
for _, host := range cluster.Hosts {
id := strfmt.UUID(uuid.New().String())
host.ID = &id
}

g := NewGenerator("", workDir, installerCacheDir, cluster, "", "", "", "", nil, log,
mockOperatorManager, mockProviderRegistry, "", "", 5).(*installerGenerator)

err := g.createHostIgnitions()
Expect(err).NotTo(HaveOccurred())

for _, host := range cluster.Hosts {
ignBytes, err := os.ReadFile(filepath.Join(workDir, fmt.Sprintf("%s-%s.ign", host.Role, host.ID)))
Expect(err).NotTo(HaveOccurred())
config, _, err := config_32.Parse(ignBytes)
Expect(err).NotTo(HaveOccurred())

By("Validating nodeip hint was added")
var f *config_32_types.File
for fileidx, file := range config.Storage.Files {
if file.Node.Path == nodeIpHintFile {
f = &config.Storage.Files[fileidx]
break
}
}
Expect(f).NotTo(BeNil())
Expect(*f.Node.User.Name).To(Equal("root"))
Expect(*f.FileEmbedded1.Contents.Source).To(Equal(fmt.Sprintf("data:,KUBELET_NODEIP_HINT=%s", "3.3.3.0")))
Expect(*f.FileEmbedded1.Mode).To(Equal(420))
Expect(*f.Node.Overwrite).To(Equal(true))

}
})

It("MULTI NODE: no nodeip hint file", func() {
cluster.Hosts = []*models.Host{
{
RequestedHostname: "master0.example.com",
Role: models.HostRoleMaster,
},
}
cluster.HighAvailabilityMode = swag.String(models.ClusterHighAvailabilityModeFull)
cluster.MachineNetworks = network.CreateMachineNetworksArray("3.3.3.0/24")

// create an ID for each host
for _, host := range cluster.Hosts {
id := strfmt.UUID(uuid.New().String())
host.ID = &id
}

g := NewGenerator("", workDir, installerCacheDir, cluster, "", "", "", "", nil, log,
mockOperatorManager, mockProviderRegistry, "", "", 5).(*installerGenerator)

err := g.createHostIgnitions()
Expect(err).NotTo(HaveOccurred())

for _, host := range cluster.Hosts {
ignBytes, err := os.ReadFile(filepath.Join(workDir, fmt.Sprintf("%s-%s.ign", host.Role, host.ID)))
Expect(err).NotTo(HaveOccurred())
config, _, err := config_32.Parse(ignBytes)
Expect(err).NotTo(HaveOccurred())

By("Validating nodeip hint was not added")
var f *config_32_types.File
for fileidx, file := range config.Storage.Files {
if file.Node.Path == nodeIpHintFile {
f = &config.Storage.Files[fileidx]
break
}
}
Expect(f).To(BeNil())
}
})
})

It("applies overrides correctly", func() {
hostID := strfmt.UUID(uuid.New().String())
cluster.Hosts = []*models.Host{{
Expand All @@ -593,7 +681,7 @@ var _ = Describe("createHostIgnitions", func() {
g := NewGenerator("", workDir, installerCacheDir, cluster, "", "", "", "", nil, log,
mockOperatorManager, mockProviderRegistry, "", "", 5).(*installerGenerator)

err := g.createHostIgnitions("http://www.example.com:6008", auth.TypeNone)
err := g.createHostIgnitions()
Expect(err).NotTo(HaveOccurred())

ignBytes, err := os.ReadFile(filepath.Join(workDir, fmt.Sprintf("%s-%s.ign", models.HostRoleMaster, hostID)))
Expand Down Expand Up @@ -667,7 +755,7 @@ spec:
mockS3Client.EXPECT().ListObjectsByPrefix(gomock.Any(), gomock.Any()).Return([]string{"mcp.yaml"}, nil)
mockS3Client.EXPECT().ListObjectsByPrefix(gomock.Any(), gomock.Any()).Return(nil, nil)
mockS3Client.EXPECT().Download(gomock.Any(), gomock.Any()).Return(io.NopCloser(strings.NewReader(mcp)), int64(0), nil)
err := g.writeSingleHostFile(cluster.Hosts[0], workerIgn, g.workDir, "http://www.example.com:6008", "", auth.TypeNone)
err := g.writeSingleHostFile(cluster.Hosts[0], workerIgn, g.workDir)
Expect(err).NotTo(HaveOccurred())

ignBytes, err := os.ReadFile(filepath.Join(workDir, fmt.Sprintf("%s-%s.ign", models.HostRoleWorker, hostID)))
Expand All @@ -694,7 +782,7 @@ spec:
mockS3Client.EXPECT().ListObjectsByPrefix(gomock.Any(), gomock.Any()).Return([]string{"mcp.yaml"}, nil)
mockS3Client.EXPECT().ListObjectsByPrefix(gomock.Any(), gomock.Any()).Return(nil, nil)
mockS3Client.EXPECT().Download(gomock.Any(), gomock.Any()).Return(io.NopCloser(strings.NewReader(mc)), int64(0), nil)
err := g.writeSingleHostFile(cluster.Hosts[0], workerIgn, g.workDir, "http://www.example.com:6008", "", auth.TypeNone)
err := g.writeSingleHostFile(cluster.Hosts[0], workerIgn, g.workDir)
Expect(err).To(HaveOccurred())
})
})
Expand Down
74 changes: 0 additions & 74 deletions internal/network/manifests_generator.go
View file
Expand Up @@ -6,7 +6,6 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"net"
"strings"
"text/template"

Expand All @@ -28,7 +27,6 @@ import (
type ManifestsGeneratorAPI interface {
AddChronyManifest(ctx context.Context, log logrus.FieldLogger, c *common.Cluster) error
AddDnsmasqForSingleNode(ctx context.Context, log logrus.FieldLogger, c *common.Cluster) error
AddNodeIpHint(ctx context.Context, log logrus.FieldLogger, cluster *common.Cluster) error
AddTelemeterManifest(ctx context.Context, log logrus.FieldLogger, c *common.Cluster) error
AddSchedulableMastersManifest(ctx context.Context, log logrus.FieldLogger, c *common.Cluster) error
AddDiskEncryptionManifest(ctx context.Context, log logrus.FieldLogger, c *common.Cluster) error
Expand Down Expand Up @@ -535,75 +533,3 @@ func NewConfig() (*Config, error) {
}
return &networkCfg, nil
}

const nodeIpHint = `
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
labels:
machineconfiguration.openshift.io/role: {{.ROLE}}
name: 10-{{.ROLE}}s-node-ip-hint
spec:
config:
ignition:
version: 3.1.0
storage:
files:
- contents:
source: data:text/plain;charset=utf-8;base64,{{.NODE_IP_CONTENT}}
verification: {}
filesystem: root
mode: 420
path: /etc/default/nodeip-configuration
`

// Add node ip hint (is supported from 4.10 but it makes no harm to push this file to any version)
// it will allow us to tell to node-ip script which ip kubelet should run with
// https://github.com/openshift/machine-config-operator/commit/a0c9a3caa54018eb89eb5bdd6ec1b8fbf97f6fb7
func (m *ManifestsGenerator) AddNodeIpHint(ctx context.Context, log logrus.FieldLogger, cluster *common.Cluster) error {
if !common.IsSingleNodeCluster(cluster) {
return nil
}

if hintSupported, err := common.VersionGreaterOrEqual(cluster.OpenshiftVersion, "4.10.15"); err != nil || !hintSupported {
return err
}

if !IsMachineCidrAvailable(cluster) {
return fmt.Errorf("node-ip-hint allowed only if machine network was configured")
}

log.Infof("Adding node add ip hint manifests")
for _, role := range []models.HostRole{models.HostRoleMaster, models.HostRoleWorker} {
filename := fmt.Sprintf("node-ip-hint-%s.yaml", role)
content, err := createNodeIpHintContent(log, cluster, string(role))
if err != nil {
log.WithError(err).Errorf("Failed to create node ip hint manifest")
return err
}

if err := m.createManifests(ctx, cluster, filename, content); err != nil {
return err
}
}
return nil
}

func createNodeIpHintContent(log logrus.FieldLogger, cluster *common.Cluster, role string) ([]byte, error) {
log.Infof("Creating content for node-ip-hint manifest")
machineCidr := cluster.MachineNetworks[0]
ip, _, err := net.ParseCIDR(string(machineCidr.Cidr))
if err != nil {
log.WithError(err).Warn("Failed to parse machine cidr for node ip hint content")
return nil, err
}

content := fmt.Sprintf("KUBELET_NODEIP_HINT=%s", ip)

var manifestParams = map[string]interface{}{
"NODE_IP_CONTENT": base64.StdEncoding.EncodeToString([]byte(content)),
"ROLE": role,
}

return fillTemplate(manifestParams, nodeIpHint, log)
}

0 comments on commit 9cd73e3

Please sign in to comment.