Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-4.11] OCPBUGS-2080: ztp: default to split DU MachineConfig CRs and allow splitting as opt-in feature #1288

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions ztp/siteconfig-generator/siteConfig/siteConfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ type Clusters struct {
NumWorkers uint8
ClusterType string
CrTemplates map[string]string `yaml:"crTemplates"`

// optional: merge MachineConfigs into a single CR
MergeDefaultMachineConfigs bool `yaml:"mergeDefaultMachineConfigs"`
}

// Provide custom YAML unmarshal for Clusters which provides default values
Expand Down
13 changes: 8 additions & 5 deletions ztp/siteconfig-generator/siteConfig/siteConfigBuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,11 +463,14 @@ func (scbuilder *SiteConfigBuilder) getExtraManifest(dataMap map[string]interfac
return dataMap, err
}

// merge the pre-defined manifests
dataMap, err = MergeManifests(dataMap, doNotMerge)
if err != nil {
log.Printf("Error could not merge extra-manifest %s.%s %s\n", clusterSpec.ClusterName, clusterSpec.ExtraManifestPath, err)
return dataMap, err
// check if user wants to merge machineconfigs
if clusterSpec.MergeDefaultMachineConfigs {
// merge the pre-defined manifests
dataMap, err = MergeManifests(dataMap, doNotMerge)
if err != nil {
log.Printf("Error could not merge extra-manifest %s.%s %s\n", clusterSpec.ClusterName, clusterSpec.ExtraManifestPath, err)
return dataMap, err
}
}

return dataMap, nil
Expand Down
108 changes: 108 additions & 0 deletions ztp/siteconfig-generator/siteConfig/siteConfigBuilder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ spec:
additionalNTPSources:
- NTP.server1
- 10.16.231.22
mergeDefaultMachineConfigs: true
nodes:
- hostName: "node1"
biosConfigRef:
Expand Down Expand Up @@ -101,6 +102,7 @@ spec:
- cidr: 10.16.231.0/24
serviceNetwork:
- 172.30.0.0/16
mergeDefaultMachineConfigs: true
nodes:
- hostName: "node1"
nodeNetwork:
Expand Down Expand Up @@ -1077,3 +1079,109 @@ spec:
}

}

func Test_mergeDefaultMachineConfigs(t *testing.T) {
configSplitYaml := `
mergeDefaultMachineConfigs: %s
`
const s = `
spec:
clusterImageSetNameRef: "openshift-v4.8.0"
clusters:
- clusterName: "cluster1"
%s
nodes:
- hostName: "node1"
`

type args struct {
configSplit string
}
tests := []struct {
name string
args args
want string
wantErrMsg string
wantErr bool
}{
{
name: "split when mergeDefaultMachineConfigs is false",
wantErr: false,
args: args{
configSplit: fmt.Sprintf(configSplitYaml, `false`),
},
want: "testdata/SplitAndMergeMachineConfigExpected/splitMC.yaml",
},
{
name: "split without the presence of the the bool for mergeDefaultMachineConfigs",
wantErr: false,
args: args{
configSplit: fmt.Sprintf(configSplitYaml, ``),
},
want: "testdata/SplitAndMergeMachineConfigExpected/splitMC.yaml",
},
{
name: "split without the presence of mergeDefaultMachineConfigs",
wantErr: false,
args: args{
configSplit: "",
},
want: "testdata/SplitAndMergeMachineConfigExpected/splitMC.yaml",
},
{
name: "merge when mergeDefaultMachineConfigs set to true",
wantErr: false,
args: args{
configSplit: fmt.Sprintf(configSplitYaml, `true`),
},
want: "testdata/SplitAndMergeMachineConfigExpected/mergeMC.yaml",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
sc := SiteConfig{}
scString := fmt.Sprintf(s, tt.args.configSplit)

err := yaml.Unmarshal([]byte(scString), &sc)
if !cmp.Equal(err, nil) {
t.Errorf("Test_legacyConfigSplitMachineConfigCRs() unmarshall err got = %v, want %v", err.Error(), "no error")
t.FailNow()
}

scBuilder, err := NewSiteConfigBuilder()
scBuilder.SetLocalExtraManifestPath("testdata/source-cr-extra-manifest-copy")
clustersCRs, err := scBuilder.Build(sc)
assert.NoError(t, err)
assert.Equal(t, len(clustersCRs), len(sc.Spec.Clusters))

var outputBuffer bytes.Buffer
for _, clusterCRs := range clustersCRs {
for _, clusterCR := range clusterCRs {
cr, err := yaml.Marshal(clusterCR)
assert.NoError(t, err)

outputBuffer.Write(Separator)
outputBuffer.Write(cr)
}
}
outputStr := outputBuffer.String()
outputBuffer.Reset()
if outputStr == "" && len(err.Error()) > 0 {
t.Errorf("unable to create SC = %v, want %v", err.Error(), "no error")
t.FailNow()
}

filesData, err := ReadFile(tt.want)
if tt.wantErr {
if !cmp.Equal(outputStr, tt.wantErrMsg) {
t.Errorf("Test_MergeDefaultMachineConfigs() error case got = %v, want %v", outputStr, tt.wantErrMsg)
}
} else {
if !cmp.Equal(outputStr, string(filesData)) {
t.Errorf("Test_MergeDefaultMachineConfigs() got = %v, want %v", outputStr, string(filesData))
}
}
})
}

}

Large diffs are not rendered by default.