Skip to content

Commit

Permalink
Use a struct array instead of map when creating new ignitions
Browse files Browse the repository at this point in the history
A map doesn't guarantee order when we are creating new ignitions.
When we update the image CR with blocked registries, the ctrcfg
controller needs to update two files registries.conf and policy.json.
Since we get an update from the image CR about every 20 mins, we compare
the semantics to see if anything has changed. But since the order is not
guaranteed, the controller might think that the semantics is not equal
even if nothing in the data changed. Hence another MC is created, and
everytime we get an update the MC applied to the nodes keeps flipping
back and forth for the 2 possible orders causing the nodes to reboot a
bunch of times. So move to using a struct array to ensure the order is
always the same and we don't have two similar MCs being created.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
  • Loading branch information
umohnani8 committed Apr 14, 2020
1 parent f6d1fe7 commit 0ecbbde
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -548,9 +548,10 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error {
tempIgnCfg := ctrlcommon.NewIgnConfig()
mc = mtmpl.MachineConfigFromIgnConfig(role, managedKey, &tempIgnCfg)
}
mc.Spec.Config = createNewIgnition(map[string][]byte{
storageConfigPath: storageTOML,
crioConfigPath: crioTOML,

mc.Spec.Config = createNewIgnition([]ignitionConfig{
{filePath: storageConfigPath, data: storageTOML},
{filePath: crioConfigPath, data: crioTOML},
})

mc.SetAnnotations(map[string]string{
Expand Down Expand Up @@ -750,9 +751,9 @@ func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.Contr
return nil, fmt.Errorf("could not update policy json with new changes: %v", err)
}
}
registriesIgn := createNewIgnition(map[string][]byte{
registriesConfigPath: registriesTOML,
policyConfigPath: policyJSON,
registriesIgn := createNewIgnition([]ignitionConfig{
{filePath: registriesConfigPath, data: registriesTOML},
{filePath: policyConfigPath, data: policyJSON},
})
return &registriesIgn, nil
}
Expand Down
18 changes: 13 additions & 5 deletions pkg/controller/container-runtime-config/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,26 +59,34 @@ type tomlConfigCRIO struct {
} `toml:"crio"`
}

// ignitionConfig is a struct that holds the filepath and date of the various configs
// Using a struct array ensures that the order of the ignition files always stay the same
// ensuring that double MCs are not created due to a change in the order
type ignitionConfig struct {
filePath string
data []byte
}

type updateConfigFunc func(data []byte, internal *mcfgv1.ContainerRuntimeConfiguration) ([]byte, error)

// createNewIgnition takes a map where the key is the path of the file, and the value is the
// new data in the form of a byte array. The function returns the ignition config with the
// updated data.
func createNewIgnition(configs map[string][]byte) igntypes.Config {
func createNewIgnition(configs []ignitionConfig) igntypes.Config {
tempIgnConfig := ctrlcommon.NewIgnConfig()
mode := 0644
// Create ignitions
for filePath, data := range configs {
for _, ignConf := range configs {
// If the file is not included, the data will be nil so skip over
if data == nil {
if ignConf.data == nil {
continue
}
configdu := dataurl.New(data, "text/plain")
configdu := dataurl.New(ignConf.data, "text/plain")
configdu.Encoding = dataurl.EncodingASCII
configTempFile := igntypes.File{
Node: igntypes.Node{
Filesystem: "root",
Path: filePath,
Path: ignConf.filePath,
},
FileEmbedded1: igntypes.FileEmbedded1{
Mode: &mode,
Expand Down

0 comments on commit 0ecbbde

Please sign in to comment.