Skip to content

Commit

Permalink
MGMT-13620: Make worker to use custom MCP if it was set as part of th…
Browse files Browse the repository at this point in the history 
…e host DB record (openshift#4976)

The pointer ignition contains a link to pull the ignition associated
with the MCP(i.e master/worker). If a worker node is associated with
custom role which is associated with custom MCP, the node will reboot
just to move to the custom MCP.  This reboot causes additional delay
during the installation process which is undesirable.
With this change, the pointer ignition is modified according to the
custom MCP name.  Thus, no extra reboots will be performed by the node
to move to the custom MCP.
  • Loading branch information
@ori-amizur
ori-amizur authored Feb 19, 2023
1 parent 8a493b1 commit 489e5e6
Show file tree
Hide file tree
Showing 2 changed files with 244 additions and 45 deletions.
190 changes: 152 additions & 38 deletions internal/ignition/ignition.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
config_latest_trans "github.com/coreos/ignition/v2/config/v3_2/translate"
config_latest_types "github.com/coreos/ignition/v2/config/v3_2/types"
"github.com/coreos/vcontext/report"
"github.com/go-openapi/strfmt"
"github.com/go-openapi/swag"
bmh_v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1"
Expand Down Expand Up @@ -1286,55 +1287,168 @@ func getBootReporterFileContent() (string, error) {
return base64.StdEncoding.EncodeToString(data), nil
}

func (g *installerGenerator) writeHostFiles(hosts []*models.Host, baseFile string, workDir string, serviceBaseURL string, authType auth.AuthType) error {
errGroup := new(errgroup.Group)
bootReporter, err := getBootReporterFileContent()
func (g *installerGenerator) getManifestContent(ctx context.Context, manifest string) (string, error) {
respBody, _, err := g.s3Client.Download(ctx, manifest)
if err != nil {
return errors.Wrap(err, "failed to read the contents of assisted-boot-reporter.sh")
return "", err
}
content, err := io.ReadAll(respBody)
if err != nil {
return "", err
}
return string(content), nil
}

for i := range hosts {
host := hosts[i]
errGroup.Go(func() error {
config, err := parseIgnitionFile(filepath.Join(workDir, baseFile))
if err != nil {
return err
func machineConfilePoolExists(manifestFname, content, poolName string) (bool, error) {
var (
manifest struct {
Kind string
Metadata *struct {
Name string
}
pullSecretToken, err := clusterPkg.AgentToken(g.cluster, authType)
if err != nil {
return err
}
contents := fmt.Sprintf(assistedBootReporterunitTemplate, strings.TrimSpace(serviceBaseURL), pullSecretToken, host.ClusterID, host.ID, 5, 60)
setUnitInIgnition(config, contents, "assisted-boot-reporter.service", true)
}
err error
)
ext := filepath.Ext(manifestFname)
switch ext {
case ".yml", ".yaml":
err = yaml.Unmarshal([]byte(content), &manifest)
case ".json":
err = json.Unmarshal([]byte(content), &manifest)
default:
return false, nil
}
if err != nil {
return false, err
}
return manifest.Kind == "MachineConfigPool" && manifest.Metadata != nil && manifest.Metadata.Name == poolName, nil
}

hostname, err := hostutil.GetCurrentHostName(host)
if err != nil {
return errors.Wrapf(err, "failed to get hostname for host %s", host.ID)
}
func (g *installerGenerator) clusterHasMCP(poolName string, clusterId *strfmt.UUID) (bool, error) {
var err error
ctx := context.Background()
manifestList, err := manifests.GetClusterManifests(ctx, clusterId, g.s3Client)
if err != nil {
return false, err
}
for _, manifest := range manifestList {
content, err := g.getManifestContent(ctx, manifest)
if err != nil {
return false, err
}
exists, err := machineConfilePoolExists(manifest, content, poolName)
if err != nil {
return false, err
}
if exists {
return true, nil
}
}
return false, nil
}

setFileInIgnition(config, "/etc/hostname", fmt.Sprintf("data:,%s", hostname), false, 420, true)
func (g *installerGenerator) updatePointerIgnitionMCP(poolName string, ignitionStr string) (string, error) {
config, err := ParseToLatest([]byte(ignitionStr))
if err != nil {
return "", err
}
for i := range config.Ignition.Config.Merge {
r := &config.Ignition.Config.Merge[i]
if r.Source != nil {
r.Source = swag.String(strings.Replace(swag.StringValue(r.Source), "config/worker", "config/"+poolName, 1))
}
}
b, err := json.Marshal(config)
if err != nil {
return "", err
}
return string(b), nil
}

setFileInIgnition(config, "/usr/local/bin/assisted-boot-reporter.sh", fmt.Sprintf("data:text/plain;charset=utf-8;base64,%s", bootReporter), false, 0700, true)
func (g *installerGenerator) modifyPointerIgnitionMCP(poolName string, ignitionStr string, clusterId *strfmt.UUID) (string, error) {
var (
mcpExists bool
err error
ret string
)
mcpExists, err = g.clusterHasMCP(poolName, clusterId)
if err != nil {
g.log.WithError(err).Errorf("failed to find if machine config pool %s exists", poolName)
return "", err
}
if mcpExists {
ret, err = g.updatePointerIgnitionMCP(poolName, ignitionStr)
if err != nil {
g.log.WithError(err).Errorf("failed to update pointer ignition for pool %s", poolName)
return "", err
}
return ret, nil
}
return "", errors.Errorf("machine config pool %s was not found", poolName)
}

configBytes, err := json.Marshal(config)
if err != nil {
return err
}
func (g *installerGenerator) writeSingleHostFile(host *models.Host, baseFile string, workDir, serviceBaseURL, bootReporter string, authType auth.AuthType) error {
config, err := parseIgnitionFile(filepath.Join(workDir, baseFile))
if err != nil {
return err
}
pullSecretToken, err := clusterPkg.AgentToken(g.cluster, authType)
if err != nil {
return err
}
contents := fmt.Sprintf(assistedBootReporterunitTemplate, strings.TrimSpace(serviceBaseURL), pullSecretToken, host.ClusterID, host.ID, 5, 60)
setUnitInIgnition(config, contents, "assisted-boot-reporter.service", true)

if host.IgnitionConfigOverrides != "" {
merged, mergeErr := MergeIgnitionConfig(configBytes, []byte(host.IgnitionConfigOverrides))
if mergeErr != nil {
return errors.Wrapf(mergeErr, "failed to apply ignition config overrides for host %s", host.ID)
}
configBytes = []byte(merged)
}
hostname, err := hostutil.GetCurrentHostName(host)
if err != nil {
return errors.Wrapf(err, "failed to get hostname for host %s", host.ID)
}

err = os.WriteFile(filepath.Join(workDir, hostutil.IgnitionFileName(host)), configBytes, 0600)
if err != nil {
return errors.Wrapf(err, "failed to write ignition for host %s", host.ID)
}
setFileInIgnition(config, "/etc/hostname", fmt.Sprintf("data:,%s", hostname), false, 420, true)

return nil
setFileInIgnition(config, "/usr/local/bin/assisted-boot-reporter.sh", fmt.Sprintf("data:text/plain;charset=utf-8;base64,%s", bootReporter), false, 0700, true)

configBytes, err := json.Marshal(config)
if err != nil {
return err
}

if host.IgnitionConfigOverrides != "" {
merged, mergeErr := MergeIgnitionConfig(configBytes, []byte(host.IgnitionConfigOverrides))
if mergeErr != nil {
return errors.Wrapf(mergeErr, "failed to apply ignition config overrides for host %s", host.ID)
}
configBytes = []byte(merged)
}

if host.Role == models.HostRoleWorker && host.MachineConfigPoolName != "" {
var override string
override, err = g.modifyPointerIgnitionMCP(host.MachineConfigPoolName, string(configBytes), host.ClusterID)
if err != nil {
return errors.Wrapf(err, "failed to set machine config pool %s to pointer ignition for host %s",
host.MachineConfigPoolName, host.ID.String())
}
configBytes = []byte(override)
}

err = os.WriteFile(filepath.Join(workDir, hostutil.IgnitionFileName(host)), configBytes, 0600)
if err != nil {
return errors.Wrapf(err, "failed to write ignition for host %s", host.ID)
}

return nil
}

func (g *installerGenerator) writeHostFiles(hosts []*models.Host, baseFile string, workDir string, serviceBaseURL string, authType auth.AuthType) error {
errGroup := new(errgroup.Group)
bootReporter, err := getBootReporterFileContent()
if err != nil {
return errors.Wrap(err, "failed to read the contents of assisted-boot-reporter.sh")
}
for i := range hosts {
host := hosts[i]
errGroup.Go(func() error {
return g.writeSingleHostFile(host, baseFile, workDir, serviceBaseURL, bootReporter, authType)
})
}

Expand Down
99 changes: 92 additions & 7 deletions internal/ignition/ignition_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ SV4bRR9i0uf+xQ/oYRvugQ25Q7EahO5hJIWRf4aULbk36Zpw3++v2KFnF26zqwB6

// TODO(deprecate-ignition-3.1.0)
var _ = Describe("createHostIgnitions", func() {
const masterIgn = `{
const testMasterIgn = `{
"ignition": {
"config": {
"merge": [
Expand Down Expand Up @@ -465,7 +465,7 @@ var _ = Describe("createHostIgnitions", func() {
]
}
}`
const workerIgn = `{
const testWorkerIgn = `{
"ignition": {
"config": {
"merge": [
Expand All @@ -488,24 +488,27 @@ var _ = Describe("createHostIgnitions", func() {
}`

var (
dbName string
db *gorm.DB
dbName string
db *gorm.DB
mockS3Client *s3wrapper.MockAPI
)

BeforeEach(func() {
masterPath := filepath.Join(workDir, "master.ign")
err := os.WriteFile(masterPath, []byte(masterIgn), 0600)
err := os.WriteFile(masterPath, []byte(testMasterIgn), 0600)
Expect(err).NotTo(HaveOccurred())

workerPath := filepath.Join(workDir, "worker.ign")
err = os.WriteFile(workerPath, []byte(workerIgn), 0600)
err = os.WriteFile(workerPath, []byte(testWorkerIgn), 0600)
Expect(err).NotTo(HaveOccurred())
db, dbName = common.PrepareTestDB()

ctrl = gomock.NewController(GinkgoT())
mockS3Client = s3wrapper.NewMockAPI(ctrl)
})

AfterEach(func() {
common.DeleteTestDB(db, dbName)
ctrl.Finish()
})

Context("with multiple hosts with a hostname", func() {
Expand Down Expand Up @@ -624,6 +627,88 @@ var _ = Describe("createHostIgnitions", func() {

Expect(*exampleFile.FileEmbedded1.Contents.Source).To(Equal("data:text/plain;base64,aGVscGltdHJhcHBlZGluYXN3YWdnZXJzcGVj"))
})
Context("machine config pool", func() {
const (
mcp = `apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfigPool
metadata:
name: infra
spec:
machineConfigSelector:
matchExpressions:
- {key: machineconfiguration.openshift.io/role, operator: In, values: [worker,infra]}
maxUnavailable: null
nodeSelector:
matchLabels:
node-role.kubernetes.io/infra: ""
paused: false`

mc = `apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
labels:
machineconfiguration.openshift.io/role: infra
name: 50-infra
spec:
config:
ignition:
version: 2.2.0
storage:
files:
- contents:
source: data:,test
filesystem: root
mode: 0644
path: /etc/testinfra`
)

It("applies machine config pool correctly", func() {
hostID := strfmt.UUID(uuid.New().String())
clusterID := strfmt.UUID(uuid.New().String())
cluster.Hosts = []*models.Host{{
ID: &hostID,
ClusterID: &clusterID,
RequestedHostname: "worker0.example.com",
Role: models.HostRoleWorker,
MachineConfigPoolName: "infra",
}}

g := NewGenerator("", workDir, installerCacheDir, cluster, "", "", "", "", mockS3Client, log,
mockOperatorManager, mockProviderRegistry, "", "").(*installerGenerator)
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)
Expect(err).NotTo(HaveOccurred())

ignBytes, err := os.ReadFile(filepath.Join(workDir, fmt.Sprintf("%s-%s.ign", models.HostRoleWorker, hostID)))
Expect(err).NotTo(HaveOccurred())
config, _, err := config_32.Parse(ignBytes)
Expect(err).NotTo(HaveOccurred())
Expect(config.Ignition.Config.Merge).To(HaveLen(1))
Expect(swag.StringValue(config.Ignition.Config.Merge[0].Source)).To(HaveSuffix("config/infra"))
})

It("mcp not found", func() {
hostID := strfmt.UUID(uuid.New().String())
clusterID := strfmt.UUID(uuid.New().String())
cluster.Hosts = []*models.Host{{
ID: &hostID,
ClusterID: &clusterID,
RequestedHostname: "worker0.example.com",
Role: models.HostRoleWorker,
MachineConfigPoolName: "infra",
}}

g := NewGenerator("", workDir, installerCacheDir, cluster, "", "", "", "", mockS3Client, log,
mockOperatorManager, mockProviderRegistry, "", "").(*installerGenerator)
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)
Expect(err).To(HaveOccurred())
})
})
})

var _ = Describe("Openshift cluster ID extraction", func() {
Expand Down

0 comments on commit 489e5e6

Please sign in to comment.