Skip to content

Commit

Permalink
operator: update isMachineConfigurationValid to check osImageURL agai…
Browse files Browse the repository at this point in the history
…nst configmap
  • Loading branch information
kikisdeliveryservice committed May 24, 2021
1 parent c415ce6 commit db4ca30
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
15 changes: 13 additions & 2 deletions pkg/operator/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,9 @@ func (optr *Operator) allMachineConfigPoolStatus() (map[string]string, error) {
return ret, nil
}

// isMachineConfigPoolConfigurationValid returns nil, or error when the configuration of a `pool` is created by the controller at version `version`.
func isMachineConfigPoolConfigurationValid(pool *mcfgv1.MachineConfigPool, version string, machineConfigGetter func(string) (*mcfgv1.MachineConfig, error)) error {
// isMachineConfigPoolConfigurationValid returns nil, or error when the configuration of a `pool` is created by the controller at version `version` or
// when the osImageURL does not match what's in the configmap
func isMachineConfigPoolConfigurationValid(pool *mcfgv1.MachineConfigPool, version, osURL string, machineConfigGetter func(string) (*mcfgv1.MachineConfig, error)) error {
// both .status.configuration.name and .status.configuration.source must be set.
if pool.Spec.Configuration.Name == "" {
return fmt.Errorf("configuration spec for pool %s is empty: %v", pool.GetName(), machineConfigPoolStatus(pool))
Expand Down Expand Up @@ -416,6 +417,16 @@ func isMachineConfigPoolConfigurationValid(pool *mcfgv1.MachineConfigPool, versi
glog.Infof("pool %s references machine config %s, which does not declare a version", pool.GetName(), mcName)
}
}
// all MCs were generated by correct controller, but osImageURL is not a source, so let's double check here
// to cover case where hashes match but there is an upgrade and avoid race where matching hashes pass before a new config is
// rolled out
renderedMC, err := machineConfigGetter(pool.Status.Configuration.Name)
if err != nil {
return err
}
if renderedMC.Spec.OSImageURL != osURL {
return fmt.Errorf("osImageURL mismatch for %s in %s expected: %s got: %s", pool.GetName(), renderedMC.Name, osURL, renderedMC.Spec.OSImageURL)
}
return nil
}

Expand Down
7 changes: 6 additions & 1 deletion pkg/operator/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,12 @@ func (optr *Operator) syncRequiredMachineConfigPools(_ *renderConfig) error {
_, hasRequiredPoolLabel := pool.Labels[requiredForUpgradeMachineConfigPoolLabelKey]

if hasRequiredPoolLabel {
if err := isMachineConfigPoolConfigurationValid(pool, version.Hash, optr.mcLister.Get); err != nil {
opURL, err := optr.getOsImageURL(optr.namespace)
if err != nil {
glog.Errorf("Error getting configmap osImageURL: %q", err)
return false, nil
}
if err := isMachineConfigPoolConfigurationValid(pool, version.Hash, opURL, optr.mcLister.Get); err != nil {
lastErr = fmt.Errorf("pool %s has not progressed to latest configuration: %v, retrying", pool.Name, err)
glog.Info(lastErr.Error())
syncerr := optr.syncUpgradeableStatus()
Expand Down

0 comments on commit db4ca30

Please sign in to comment.