fix race between ignition payload generation and MCS configmap update

control-plane-operator/controllers/hostedcontrolplane/kcm/deployment.go

@@ -99,7 +99,7 @@ func ReconcileDeployment(deployment *appsv1.Deployment, config, rootCA, serviceS
 		deployment.Spec.Template.ObjectMeta.Annotations = map[string]string{}
 	}
 	deployment.Spec.Template.ObjectMeta.Annotations[configHashAnnotation] = util.ComputeHash(configBytes)
-	deployment.Spec.Template.ObjectMeta.Annotations[rootCAHashAnnotation] = util.HashStruct(rootCA.Data)
+	deployment.Spec.Template.ObjectMeta.Annotations[rootCAHashAnnotation] = util.HashSimple(rootCA.Data)
 
 	deployment.Spec.Template.Spec = corev1.PodSpec{
 		AutomountServiceAccountToken: pointer.Bool(false),
@@ -120,7 +120,7 @@ func ReconcileDeployment(deployment *appsv1.Deployment, config, rootCA, serviceS
 	}
 	p.DeploymentConfig.ApplyTo(deployment)
 	if serviceServingCA != nil {
-		deployment.Spec.Template.ObjectMeta.Annotations[serviceCAHashAnnotation] = util.HashStruct(serviceServingCA.Data)
+		deployment.Spec.Template.ObjectMeta.Annotations[serviceCAHashAnnotation] = util.HashSimple(serviceServingCA.Data)
 		applyServingCAVolume(&deployment.Spec.Template.Spec, serviceServingCA)
 	} else {
 		deployment.Spec.Template.ObjectMeta.Annotations[serviceCAHashAnnotation] = ""

control-plane-operator/controllers/hostedcontrolplane/mcs/params.go

@@ -9,20 +9,22 @@ import (
 	hyperv1 "github.com/openshift/hypershift/api/v1beta1"
 	"github.com/openshift/hypershift/support/config"
 	"github.com/openshift/hypershift/support/globalconfig"
+	"github.com/openshift/hypershift/support/util"
 )
 
 type MCSParams struct {
-	OwnerRef        config.OwnerRef
-	RootCA          *corev1.Secret
-	KubeletClientCA *corev1.ConfigMap
-	UserCA          *corev1.ConfigMap
-	PullSecret      *corev1.Secret
-	DNS             *configv1.DNS
-	Infrastructure  *configv1.Infrastructure
-	Network         *configv1.Network
-	Proxy           *configv1.Proxy
-	Image           *configv1.Image
-	InstallConfig   *globalconfig.InstallConfig
+	OwnerRef          config.OwnerRef
+	RootCA            *corev1.Secret
+	KubeletClientCA   *corev1.ConfigMap
+	UserCA            *corev1.ConfigMap
+	PullSecret        *corev1.Secret
+	DNS               *configv1.DNS
+	Infrastructure    *configv1.Infrastructure
+	Network           *configv1.Network
+	Proxy             *configv1.Proxy
+	Image             *configv1.Image
+	InstallConfig     *globalconfig.InstallConfig
+	ConfigurationHash string
 }
 
 func NewMCSParams(hcp *hyperv1.HostedControlPlane, rootCA, pullSecret *corev1.Secret, userCA, kubeletClientCA *corev1.ConfigMap) (*MCSParams, error) {
@@ -44,17 +46,23 @@ func NewMCSParams(hcp *hyperv1.HostedControlPlane, rootCA, pullSecret *corev1.Se
 	image := globalconfig.ImageConfig()
 	globalconfig.ReconcileImageConfig(image, hcp)
 
+	hcConfigurationHash, err := util.HashStruct(hcp.Spec.Configuration)
+	if err != nil {
+		return &MCSParams{}, fmt.Errorf("failed to hash HCP configuration: %w", err)
+	}
+
 	return &MCSParams{
-		OwnerRef:        config.OwnerRefFrom(hcp),
-		RootCA:          rootCA,
-		KubeletClientCA: kubeletClientCA,
-		UserCA:          userCA,
-		PullSecret:      pullSecret,
-		DNS:             dns,
-		Infrastructure:  infra,
-		Network:         network,
-		Proxy:           proxy,
-		Image:           image,
-		InstallConfig:   globalconfig.NewInstallConfig(hcp),
+		OwnerRef:          config.OwnerRefFrom(hcp),
+		RootCA:            rootCA,
+		KubeletClientCA:   kubeletClientCA,
+		UserCA:            userCA,
+		PullSecret:        pullSecret,
+		DNS:               dns,
+		Infrastructure:    infra,
+		Network:           network,
+		Proxy:             proxy,
+		Image:             image,
+		InstallConfig:     globalconfig.NewInstallConfig(hcp),
+		ConfigurationHash: hcConfigurationHash,
 	}, nil
 }

control-plane-operator/controllers/hostedcontrolplane/mcs/reconcile.go

@@ -62,6 +62,7 @@ func ReconcileMachineConfigServerConfig(cm *corev1.ConfigMap, p *MCSParams) erro
 	cm.Data["install-config.yaml"] = p.InstallConfig.String()
 	cm.Data["master.machineconfigpool.yaml"] = serializedMasterConfigPool
 	cm.Data["worker.machineconfigpool.yaml"] = serializedWorkerConfigPool
+	cm.Data["configuration-hash"] = p.ConfigurationHash
 	return nil
 }
 

control-plane-operator/controllers/hostedcontrolplane/oapi/deployment.go

@@ -170,7 +170,7 @@ func ReconcileDeployment(deployment *appsv1.Deployment, auditWebhookRef *corev1.
 		deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, util.BuildVolume(serviceCASignerVolume(), buildServiceCASignerVolume))
 		trustAnchorGeneratorContainer := util.FindContainer(oasTrustAnchorGenerator().Name, deployment.Spec.Template.Spec.InitContainers)
 		trustAnchorGeneratorContainer.VolumeMounts = append(trustAnchorGeneratorContainer.VolumeMounts, serviceSignerCertMount.ContainerMounts(oasTrustAnchorGenerator().Name)...)
-		deployment.Spec.Template.ObjectMeta.Annotations[serviceCAHashAnnotation] = util.HashStruct(serviceServingCA.Data)
+		deployment.Spec.Template.ObjectMeta.Annotations[serviceCAHashAnnotation] = util.HashSimple(serviceServingCA.Data)
 	} else {
 		deployment.Spec.Template.ObjectMeta.Annotations[serviceCAHashAnnotation] = ""
 	}

hypershift-operator/controllers/nodepool/nodepool_controller.go

@@ -89,6 +89,7 @@ const (
 	TokenSecretReleaseKey                     = "release"
 	TokenSecretTokenKey                       = "token"
 	TokenSecretPullSecretHashKey              = "pull-secret-hash"
+	TokenSecretHCConfigurationHashKey         = "hc-configuration-hash"
 	TokenSecretConfigKey                      = "config"
 	TokenSecretAnnotation                     = "hypershift.openshift.io/ignition-config"
 	TokenSecretIgnitionReachedAnnotation      = "hypershift.openshift.io/ignition-reached"
@@ -610,7 +611,7 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho
 	}
 
 	// Check if config needs to be updated.
-	targetConfigHash := supportutil.HashStruct(config + pullSecretName)
+	targetConfigHash := supportutil.HashSimple(config + pullSecretName)
 	isUpdatingConfig := isUpdatingConfig(nodePool, targetConfigHash)
 	if isUpdatingConfig {
 		SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{
@@ -645,7 +646,7 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho
 	}
 
 	// Signal ignition payload generation
-	targetPayloadConfigHash := supportutil.HashStruct(config + targetVersion + pullSecretName + globalConfig)
+	targetPayloadConfigHash := supportutil.HashSimple(config + targetVersion + pullSecretName + globalConfig)
 	tokenSecret := TokenSecret(controlPlaneNamespace, nodePool.Name, targetPayloadConfigHash)
 	condition, err := r.createValidGeneratedPayloadCondition(ctx, tokenSecret, nodePool.Generation)
 	if err != nil {
@@ -888,8 +889,12 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to get pull secret bytes: %w", err)
 	}
+	hcConfigurationHash, err := supportutil.HashStruct(hcluster.Spec.Configuration)
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to hash HostedCluster configuration: %w", err)
+	}
 	if result, err := r.CreateOrUpdate(ctx, r.Client, tokenSecret, func() error {
-		return reconcileTokenSecret(tokenSecret, nodePool, compressedConfig.Bytes(), pullSecretBytes)
+		return reconcileTokenSecret(tokenSecret, nodePool, compressedConfig.Bytes(), pullSecretBytes, hcConfigurationHash)
 	}); err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to reconcile token Secret: %w", err)
 	} else {
@@ -1481,7 +1486,7 @@ func reconcileTuningConfigMap(tuningConfigMap *corev1.ConfigMap, nodePool *hyper
 	return nil
 }
 
-func reconcileTokenSecret(tokenSecret *corev1.Secret, nodePool *hyperv1.NodePool, compressedConfig []byte, pullSecret []byte) error {
+func reconcileTokenSecret(tokenSecret *corev1.Secret, nodePool *hyperv1.NodePool, compressedConfig []byte, pullSecret []byte, hcConfigurationHash string) error {
 	// The token secret controller updates expired token IDs for token Secrets.
 	// When that happens the NodePool controller reconciles the userData Secret with the new token ID.
 	// Therefore, this secret is mutable.
@@ -1502,7 +1507,8 @@ func reconcileTokenSecret(tokenSecret *corev1.Secret, nodePool *hyperv1.NodePool
 		tokenSecret.Data[TokenSecretTokenKey] = []byte(uuid.New().String())
 		tokenSecret.Data[TokenSecretReleaseKey] = []byte(nodePool.Spec.Release.Image)
 		tokenSecret.Data[TokenSecretConfigKey] = compressedConfig
-		tokenSecret.Data[TokenSecretPullSecretHashKey] = []byte(supportutil.HashStruct(pullSecret))
+		tokenSecret.Data[TokenSecretPullSecretHashKey] = []byte(supportutil.HashSimple(pullSecret))
+		tokenSecret.Data[TokenSecretHCConfigurationHashKey] = []byte(hcConfigurationHash)
 	}
 	return nil
 }
@@ -2450,13 +2456,13 @@ func getName(base, suffix string, maxLength int) string {
 	if baseLength < 1 {
 		prefix := base[0:min(len(base), max(0, maxLength-9))]
 		// Calculate hash on initial base-suffix string
-		shortName := fmt.Sprintf("%s-%s", prefix, supportutil.HashStruct(name))
+		shortName := fmt.Sprintf("%s-%s", prefix, supportutil.HashSimple(name))
 		return shortName[:min(maxLength, len(shortName))]
 	}
 
 	prefix := base[0:baseLength]
 	// Calculate hash on initial base-suffix string
-	return fmt.Sprintf("%s-%s-%s", prefix, supportutil.HashStruct(base), suffix)
+	return fmt.Sprintf("%s-%s-%s", prefix, supportutil.HashSimple(base), suffix)
 }
 
 // max returns the greater of its 2 inputs
@@ -2589,7 +2595,7 @@ func machineTemplateBuilders(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.
 func generateMachineTemplateName(nodePool *hyperv1.NodePool, machineTemplateSpecJSON []byte) string {
 	// using HashStruct(machineTemplateSpecJSON) ensures a rolling upgrade is triggered
 	// by creating a new template with a differnt name if any field changes.
-	return getName(nodePool.GetName(), supportutil.HashStruct(machineTemplateSpecJSON),
+	return getName(nodePool.GetName(), supportutil.HashSimple(machineTemplateSpecJSON),
 		validation.DNS1123SubdomainMaxLength)
 }
 

ignition-server/cmd/run_local_ignitionprovider.go

@@ -106,7 +106,7 @@ func (o *RunLocalIgnitionProviderOptions) Run(ctx context.Context) error {
 		FeatureGateManifest: o.FeatureGateManifest,
 	}
 
-	payload, err := p.GetPayload(ctx, o.Image, config.String(), "")
+	payload, err := p.GetPayload(ctx, o.Image, config.String(), "", "")
 	if err != nil {
 		return err
 	}

ignition-server/controllers/local_ignitionprovider.go

@@ -76,7 +76,7 @@ type LocalIgnitionProvider struct {
 
 var _ IgnitionProvider = (*LocalIgnitionProvider)(nil)
 
-func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage string, customConfig string, pullSecretHash string) ([]byte, error) {
+func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage string, customConfig string, pullSecretHash string, hcConfigurationHash string) ([]byte, error) {
 	p.lock.Lock()
 	defer p.lock.Unlock()
 
@@ -99,7 +99,7 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage str
 	}
 
 	// Verify the pullSecret hash matches the passed-in parameter pullSecretHash to ensure the correct pull secret gets loaded into the payload
-	if pullSecretHash != "" && util.HashStruct(pullSecret) != pullSecretHash {
+	if pullSecretHash != "" && util.HashSimple(pullSecret) != pullSecretHash {
 		return nil, fmt.Errorf("pull secret does not match hash")
 	}
 
@@ -125,6 +125,11 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage str
 		return nil, fmt.Errorf("failed to get machine-config-server configmap: %w", err)
 	}
 
+	// Verify the MCS configmap is up-to-date
+	if hcConfigurationHash != "" && mcsConfig.Data["configuration-hash"] != hcConfigurationHash {
+		return nil, fmt.Errorf("machine-config-server configmap is out of date, waiting for update %s != %s", mcsConfig.Data["configuration-hash"], hcConfigurationHash)
+	}
+
 	// Look up the release image metadata
 	imageProvider, err := func() (*imageprovider.ReleaseImageProvider, error) {
 		img, err := p.ReleaseProvider.Lookup(ctx, releaseImage, pullSecret)
@@ -185,6 +190,9 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage str
 	}
 	// Extract MCS config files into the config directory
 	for name, contents := range mcsConfig.Data {
+		if name == "configuration-hash" {
+			continue
+		}
 		if err := os.WriteFile(filepath.Join(configDir, name), []byte(contents), 0644); err != nil {
 			return nil, fmt.Errorf("failed to write MCS config file %q: %w", name, err)
 		}

ignition-server/controllers/machineconfigserver_ignitionprovider.go

@@ -44,7 +44,7 @@ type MCSIgnitionProvider struct {
 	Namespace       string
 }
 
-func (p *MCSIgnitionProvider) GetPayload(ctx context.Context, releaseImage string, config string, pullSecretHash string) (payload []byte, err error) {
+func (p *MCSIgnitionProvider) GetPayload(ctx context.Context, releaseImage string, config string, pullSecretHash string, _ string) (payload []byte, err error) {
 	pullSecret := &corev1.Secret{}
 	if err := p.Client.Get(ctx, client.ObjectKey{Namespace: p.Namespace, Name: pullSecretName}, pullSecret); err != nil {
 		return nil, fmt.Errorf("failed to get pull secret: %w", err)

ignition-server/controllers/tokensecret_controller.go

@@ -23,18 +23,19 @@ import (
 )
 
 const (
-	TokenSecretReleaseKey          = "release"
-	TokenSecretConfigKey           = "config"
-	TokenSecretTokenKey            = "token"
-	TokenSecretOldTokenKey         = "old_token"
-	TokenSecretPayloadKey          = "payload"
-	TokenSecretMessageKey          = "message"
-	TokenSecretPullSecretHashKey   = "pull-secret-hash"
-	InvalidConfigReason            = "InvalidConfig"
-	TokenSecretReasonKey           = "reason"
-	TokenSecretAnnotation          = "hypershift.openshift.io/ignition-config"
-	TokenSecretNodePoolUpgradeType = "hypershift.openshift.io/node-pool-upgrade-type"
-	TokenSecretTokenGenerationTime = "hypershift.openshift.io/last-token-generation-time"
+	TokenSecretReleaseKey             = "release"
+	TokenSecretConfigKey              = "config"
+	TokenSecretTokenKey               = "token"
+	TokenSecretOldTokenKey            = "old_token"
+	TokenSecretPayloadKey             = "payload"
+	TokenSecretMessageKey             = "message"
+	TokenSecretPullSecretHashKey      = "pull-secret-hash"
+	TokenSecretHCConfigurationHashKey = "hc-configuration-hash"
+	InvalidConfigReason               = "InvalidConfig"
+	TokenSecretReasonKey              = "reason"
+	TokenSecretAnnotation             = "hypershift.openshift.io/ignition-config"
+	TokenSecretNodePoolUpgradeType    = "hypershift.openshift.io/node-pool-upgrade-type"
+	TokenSecretTokenGenerationTime    = "hypershift.openshift.io/last-token-generation-time"
 	// Set the ttl 1h above the reconcile resync period so every existing
 	// token Secret has the chance to rotate their token ID during a reconciliation cycle
 	// while the expired ones get eventually garbageCollected.
@@ -78,7 +79,7 @@ func NewPayloadStore() *ExpiringCache {
 type IgnitionProvider interface {
 	// GetPayload returns the ignition payload content for
 	// the provided release image and a config string containing 0..N MachineConfig yaml definitions.
-	GetPayload(ctx context.Context, payloadImage, config string, pullSecretHash string) ([]byte, error)
+	GetPayload(ctx context.Context, payloadImage, config string, pullSecretHash string, hcConfigurationHash string) ([]byte, error)
 }
 
 // TokenSecretReconciler watches token Secrets
@@ -258,9 +259,10 @@ func (r *TokenSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	PayloadCacheMissTotal.Inc()
 	pullSecretHash := string(tokenSecret.Data[TokenSecretPullSecretHashKey])
+	hcConfigurationHash := string(tokenSecret.Data[TokenSecretHCConfigurationHashKey])
 	payload, err := func() ([]byte, error) {
 		start := time.Now()
-		payload, err := r.IgnitionProvider.GetPayload(ctx, releaseImage, config.String(), pullSecretHash)
+		payload, err := r.IgnitionProvider.GetPayload(ctx, releaseImage, config.String(), pullSecretHash, hcConfigurationHash)
 		if err != nil {
 			return nil, fmt.Errorf("error getting ignition payload: %v", err)
 		}

ignition-server/controllers/tokensecret_controller_test.go

@@ -25,7 +25,7 @@ var (
 
 type fakeIgnitionProvider struct{}
 
-func (p *fakeIgnitionProvider) GetPayload(ctx context.Context, releaseImage string, config string, pullSecretHash string) (payload []byte, err error) {
+func (p *fakeIgnitionProvider) GetPayload(ctx context.Context, releaseImage string, config string, pullSecretHash string, hcConfigurationHash string) (payload []byte, err error) {
 	return []byte(fakePayload), nil
 }
 

support/metrics/sets.go

@@ -447,7 +447,7 @@ func RegistryOperatorRelabelConfigs(set MetricsSet) []*prometheusoperatorv1.Rela
 func SREMetricsSetConfigHash(cm *corev1.ConfigMap) string {
 	value, ok := cm.Data[SREConfigurationConfigMapKey]
 	if ok {
-		return util.HashStruct(value)
+		return util.HashSimple(value)
 	}
 	return ""
 }

support/util/util.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"crypto/tls"
 	"encoding/base64"
+	"encoding/json"
 	"fmt"
 	"hash/fnv"
 	"io"
@@ -190,14 +191,29 @@ func InsecureHTTPClient() *http.Client {
 	}
 }
 
-// HashStruct takes a value, typically a string, and returns a 32-bit FNV-1a hashed version of the value as a string
-func HashStruct(o interface{}) string {
+// HashSimple takes a value, typically a string, and returns a 32-bit FNV-1a hashed version of the value as a string
+func HashSimple(o interface{}) string {
 	hash := fnv.New32a()
 	_, _ = hash.Write([]byte(fmt.Sprintf("%v", o)))
 	intHash := hash.Sum32()
 	return fmt.Sprintf("%08x", intHash)
 }
 
+// HashStruct takes a struct and returns a 32-bit FNV-1a hashed version of the struct as a string
+// The struct is first marshalled to JSON before hashing
+func HashStruct(data interface{}) (string, error) {
+	hash := fnv.New32a()
+	jsonData, err := json.Marshal(data)
+	if err != nil {
+		return "", err
+	}
+	_, err = hash.Write(jsonData)
+	if err != nil {
+		return "", err
+	}
+	return fmt.Sprintf("%08x", hash.Sum32()), nil
+}
+
 // ConvertRegistryOverridesToCommandLineFlag converts a map of registry sources and their mirrors into a string
 func ConvertRegistryOverridesToCommandLineFlag(registryOverrides map[string]string) string {
 	var commandLineFlagArray []string