diff --git a/pkg/operator/status.go b/pkg/operator/status.go index 88d373d6db..87c83a2621 100644 --- a/pkg/operator/status.go +++ b/pkg/operator/status.go @@ -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)) @@ -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 } diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 348552f8a4..1a7d1681a4 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -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()