Skip to content

Fix hash code calculation #687

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

Merged
merged 3 commits into from
Feb 4, 2025
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions api/v1/coherence_webhook.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2024, Oracle and/or its affiliates.
* Copyright (c) 2020, 2025, Oracle and/or its affiliates.
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/
Expand Down Expand Up @@ -54,6 +54,12 @@ func (in *Coherence) Default() {
spec.SetReplicas(spec.GetReplicas())
}
SetCommonDefaults(in)

// apply a label with the hash of the spec - ths must be the last action here to make sure that
// any modifications to the spec field are included in the hash
if hash, applied := EnsureCoherenceHashLabel(in); applied {
webhookLogger.Info(fmt.Sprintf("Applied %s label", LabelCoherenceHash), "hash", hash)
}
}

// SetCommonDefaults sets defaults common to both a Job and StatefulSet
Expand Down Expand Up @@ -119,12 +125,6 @@ func SetCommonDefaults(in CoherenceResource) {

// apply the Operator version annotation
in.AddAnnotation(AnnotationOperatorVersion, operator.GetVersion())

// apply a label with the hash of the spec - ths must be the last action here to make sure that
// any modifications to the spec field are included in the hash
if hash, applied := EnsureHashLabel(in); applied {
logger.Info(fmt.Sprintf("Applied %s label", LabelCoherenceHash), "hash", hash)
}
}

// The path in this annotation MUST match the const below
Expand Down
8 changes: 7 additions & 1 deletion api/v1/coherence_webhook_job.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2024, Oracle and/or its affiliates.
* Copyright (c) 2020, 2025, Oracle and/or its affiliates.
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/
Expand Down Expand Up @@ -67,6 +67,12 @@ func (in *CoherenceJob) Default() {
}

SetCommonDefaults(in)

// apply a label with the hash of the spec - ths must be the last action here to make sure that
// any modifications to the spec field are included in the hash
if hash, applied := EnsureJobHashLabel(in); applied {
webhookLogger.Info(fmt.Sprintf("Applied %s label", LabelCoherenceHash), "hash", hash)
}
}

// The path in this annotation MUST match the const below
Expand Down
50 changes: 26 additions & 24 deletions api/v1/hasher.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, 2023, Oracle and/or its affiliates.
* Copyright (c) 2020, 2025, Oracle and/or its affiliates.
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/
Expand All @@ -8,19 +8,34 @@ package v1

import (
"encoding/binary"
"encoding/json"
"fmt"
"github.com/davecgh/go-spew/spew"
"hash"
"hash/fnv"
"k8s.io/apimachinery/pkg/util/rand"
)

func EnsureHashLabel(c CoherenceResource) (string, bool) {
func EnsureCoherenceHashLabel(c *Coherence) (string, bool) {
labels := c.GetLabels()
if labels == nil {
labels = make(map[string]string)
}
spec := c.GetSpec()
spec := c.Spec
hashNew := ComputeHash(&spec, nil)
hashCurrent, found := labels[LabelCoherenceHash]
if !found || hashCurrent != hashNew {
labels[LabelCoherenceHash] = hashNew
c.SetLabels(labels)
return hashNew, true
}
return hashCurrent, false
}

func EnsureJobHashLabel(c *CoherenceJob) (string, bool) {
labels := c.GetLabels()
if labels == nil {
labels = make(map[string]string)
}
spec := c.Spec
hashNew := ComputeHash(&spec, nil)
hashCurrent, found := labels[LabelCoherenceHash]
if !found || hashCurrent != hashNew {
Expand All @@ -33,30 +48,17 @@ func EnsureHashLabel(c CoherenceResource) (string, bool) {

// ComputeHash returns a hash value calculated from Coherence spec and
// The hash will be safe encoded to avoid bad words.
func ComputeHash(template interface{}, collisionCount *int32) string {
podTemplateSpecHasher := fnv.New32a()
DeepHashObject(podTemplateSpecHasher, template)
func ComputeHash(in interface{}, collisionCount *int32) string {
hasher := fnv.New32a()
b, _ := json.Marshal(in)
_, _ = hasher.Write(b)

// Add collisionCount in the hash if it exists.
if collisionCount != nil {
collisionCountBytes := make([]byte, 8)
binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount))
_, _ = podTemplateSpecHasher.Write(collisionCountBytes)
_, _ = hasher.Write(collisionCountBytes)
}

return rand.SafeEncodeString(fmt.Sprint(podTemplateSpecHasher.Sum32()))
}

// DeepHashObject writes specified object to hash using the spew library
// which follows pointers and prints actual values of the nested objects
// ensuring the hash does not change when a pointer changes.
func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) {
hasher.Reset()
printer := spew.ConfigState{
Indent: " ",
SortKeys: true,
DisableMethods: true,
SpewKeys: true,
}
_, _ = printer.Fprintf(hasher, "%#v", objectToWrite)
return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32()))
}
7 changes: 4 additions & 3 deletions api/v1/hasher_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2024, Oracle and/or its affiliates.
* Copyright (c) 2020, 2025, Oracle and/or its affiliates.
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/
Expand All @@ -26,10 +26,11 @@ func TestHash(t *testing.T) {
Spec: spec,
}

coh.EnsureHashLabel(deployment)
coh.EnsureCoherenceHashLabel(deployment)

// If this test fails you have probably added a new field to CoherenceResourceSpec
// This will break backwards compatibility. This field needs to be added to
// both CoherenceStatefulSetResourceSpec and CoherenceJobResourceSpec instead
g.Expect(deployment.GetLabels()["coherence-hash"]).To(Equal("5cb9fd9f96"))
//g.Expect(deployment.GetLabels()["coherence-hash"]).To(Equal("5cb9fd9f96"))
g.Expect(deployment.GetLabels()["coherence-hash"]).To(Equal("5859f96865"))
}
47 changes: 10 additions & 37 deletions controllers/coherence_controller.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2024, Oracle and/or its affiliates.
* Copyright (c) 2020, 2025, Oracle and/or its affiliates.
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/
Expand Down Expand Up @@ -208,37 +208,12 @@ func (in *CoherenceReconciler) Reconcile(ctx context.Context, request ctrl.Reque
}

hash := deployment.GetLabels()[coh.LabelCoherenceHash]
storeHash, _ := storage.GetHash()
var desiredResources coh.Resources

storeHash, found := storage.GetHash()
if !found || storeHash != hash || deployment.Status.Phase != coh.ConditionTypeReady {
// Storage state was saved with no hash or a different hash so is not in the desired state
// or the Coherence resource is not in the Ready state
// Create the desired resources the deployment
if desiredResources, err = deployment.CreateKubernetesResources(); err != nil {
return in.HandleErrAndRequeue(ctx, err, nil, fmt.Sprintf(createResourcesFailedMessage, request.Name, request.Namespace, err), in.Log)
}

if found {
// The "storeHash" is not "", so it must have been processed by the Operator (could have been a previous version).
// There was a bug prior to 3.2.8 where the hash was calculated at the wrong point in the defaulting web-hook,
// so the "currentHash" may be wrong, and hence differ from the recalculated "hash".
if deployment.IsBeforeVersion("3.3.0") {
// the AnnotationOperatorVersion annotation was added in the 3.2.8 web-hook, so if it is missing
// the Coherence resource was added or updated prior to 3.2.8
// In this case we just ignore the difference in hash.
// There is an edge case where the Coherence resource could have legitimately been updated whilst
// the Operator and web-hooks were uninstalled. In that case we would ignore the update until another
// update is made. The simplest way for the customer to work around this is to add the
// AnnotationOperatorVersion annotation with some value, which will then be overwritten by the web-hook
// and the Coherence resource will be correctly processes.
desiredResources = storage.GetLatest()
log.Info("Ignoring hash difference for pre-3.2.8 resource", "hash", hash, "store", storeHash)
}
}
} else {
// storage state was saved with the current hash so is already in the desired state
desiredResources = storage.GetLatest()
desiredResources, err = checkCoherenceHash(deployment, storage, log)
if err != nil {
return in.HandleErrAndRequeue(ctx, err, nil, fmt.Sprintf(createResourcesFailedMessage, request.Name, request.Namespace, err), in.Log)
}

// create the result
Expand Down Expand Up @@ -283,9 +258,6 @@ func (in *CoherenceReconciler) Reconcile(ctx context.Context, request ctrl.Reque
}
return reconcile.Result{}, fmt.Errorf("one or more secondary resource reconcilers failed to reconcile")
}
//} else {
// log.Info("Skipping updates for Coherence resource, annotation " + coh.AnnotationOperatorIgnore + " is set to true")
//}

// if replica count is zero update the status to Stopped
if deployment.GetReplicas() == 0 {
Expand Down Expand Up @@ -340,6 +312,7 @@ func (in *CoherenceReconciler) SetupWithManager(mgr ctrl.Manager, cs clients.Cli
func (in *CoherenceReconciler) GetReconciler() reconcile.Reconciler { return in }

// ensureHashApplied ensures that the hash label is present in the Coherence resource, patching it if required
// Returns true if the hash label was applied to the Coherence resource, or false if the label was already present
func (in *CoherenceReconciler) ensureHashApplied(ctx context.Context, c *coh.Coherence) (bool, error) {
currentHash := ""
labels := c.GetLabels()
Expand All @@ -349,14 +322,14 @@ func (in *CoherenceReconciler) ensureHashApplied(ctx context.Context, c *coh.Coh

// Re-fetch the Coherence resource to ensure we have the most recent copy
latest := c.DeepCopy()
hash, _ := coh.EnsureHashLabel(latest)
hash, _ := coh.EnsureCoherenceHashLabel(latest)

if currentHash != hash {
if c.IsBeforeVersion("3.3.0") {
// Before 3.3.0 there was a bug calculating the has in the defaulting web-hook
if c.IsBeforeVersion("3.4.2") {
// Before 3.4.2 there was a bug calculating the hash in the defaulting web-hook
// This would cause the hashes to be different here, when in fact they should not be
// If the Coherence resource being processes has no version annotation, or a version
// prior to 3.3.0 then we return as if the hashes matched
// prior to 3.4.2 then we return as if the hashes matched
if labels == nil {
labels = make(map[string]string)
}
Expand Down
27 changes: 10 additions & 17 deletions controllers/coherencejob_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,19 +160,12 @@ func (in *CoherenceJobReconciler) ReconcileDeployment(ctx context.Context, reque
}

hash := deployment.GetLabels()[coh.LabelCoherenceHash]
storeHash, _ := storage.GetHash()
var desiredResources coh.Resources

storeHash, found := storage.GetHash()
if !found || storeHash != hash || status.Phase != coh.ConditionTypeReady {
// Storage state was saved with no hash or a different hash so is not in the desired state
// or the CoherenceJob resource is not in the Ready state
// Create the desired resources the deployment
if desiredResources, err = deployment.CreateKubernetesResources(); err != nil {
return in.HandleErrAndRequeue(ctx, err, nil, fmt.Sprintf(createResourcesFailedMessage, request.Name, request.Namespace, err), in.Log)
}
} else {
// storage state was saved with the current hash so is already in the desired state
desiredResources = storage.GetLatest()
desiredResources, err = checkJobHash(deployment, storage, log)
if err != nil {
return in.HandleErrAndRequeue(ctx, err, nil, fmt.Sprintf(createResourcesFailedMessage, request.Name, request.Namespace, err), in.Log)
}

// create the result
Expand Down Expand Up @@ -271,23 +264,23 @@ func (in *CoherenceJobReconciler) SetupWithManager(mgr ctrl.Manager, cs clients.
func (in *CoherenceJobReconciler) GetReconciler() reconcile.Reconciler { return in }

// ensureHashApplied ensures that the hash label is present in the CoherenceJob resource, patching it if required
func (in *CoherenceJobReconciler) ensureHashApplied(ctx context.Context, c coh.CoherenceResource) (bool, error) {
func (in *CoherenceJobReconciler) ensureHashApplied(ctx context.Context, c *coh.CoherenceJob) (bool, error) {
currentHash := ""
labels := c.GetLabels()
if len(labels) > 0 {
currentHash = labels[coh.LabelCoherenceHash]
}

// Re-fetch the CoherenceJob resource to ensure we have the most recent copy
latest := c.DeepCopyResource()
hash, _ := coh.EnsureHashLabel(latest)
latest := c.DeepCopy()
hash, _ := coh.EnsureJobHashLabel(latest)

if currentHash != hash {
if c.IsBeforeVersion("3.2.8") {
// Before 3.2.8 there was a bug calculating the has in the defaulting web-hook
if c.IsBeforeVersion("3.4.2") {
// Before 3.4.2 there was a bug calculating the has in the defaulting web-hook
// This would cause the hashes to be different here, when in fact they should not be
// If the CoherenceJob resource being processes has no version annotation, or a version
// prior to 3.2.8 then we return as if the hashes matched
// prior to 3.4.2 then we return as if the hashes matched
if labels == nil {
labels = make(map[string]string)
}
Expand Down
60 changes: 60 additions & 0 deletions controllers/common.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright (c) 2020, 2025, Oracle and/or its affiliates.
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/

package controllers

import (
"github.com/go-logr/logr"
coh "github.com/oracle/coherence-operator/api/v1"
"github.com/oracle/coherence-operator/pkg/utils"
)

func checkCoherenceHash(deployment *coh.Coherence, storage utils.Storage, log logr.Logger) (coh.Resources, error) {
return checkHash(deployment, deployment.Status.Phase, storage, log)
}

func checkJobHash(deployment *coh.CoherenceJob, storage utils.Storage, log logr.Logger) (coh.Resources, error) {
return checkHash(deployment, deployment.Status.Phase, storage, log)
}

func checkHash(deployment coh.CoherenceResource, phase coh.ConditionType, storage utils.Storage, log logr.Logger) (coh.Resources, error) {
hash := deployment.GetLabels()[coh.LabelCoherenceHash]
var desiredResources coh.Resources
var err error

storeHash, found := storage.GetHash()
if !found || storeHash != hash || phase != coh.ConditionTypeReady {
// Storage state was saved with no hash or a different hash so is not in the desired state
// or the Coherence resource is not in the Ready state
// Create the desired resources the deployment
if desiredResources, err = deployment.CreateKubernetesResources(); err != nil {
return desiredResources, err
}

if found {
// The "storeHash" is not "", so it must have been processed by the Operator (could have been a previous version).
// There was a bug prior to 3.4.2 where the hash was calculated at the wrong point in the defaulting web-hook,
// and the has used only a portion of the spec, so the "currentHash" may be wrong, and hence differ from the
// recalculated "hash".
if deployment.IsBeforeVersion("3.4.2") {
// the AnnotationOperatorVersion annotation was added in the 3.2.8 web-hook, so if it is missing
// the Coherence resource was added or updated prior to 3.2.8, or the version is present but is
// prior to 3.4.2. In this case we just ignore the difference in hash.
// There is an edge case where the Coherence resource could have legitimately been updated whilst
// the Operator and web-hooks were uninstalled. In that case we would ignore the update until another
// update is made. The simplest way for the customer to work around this is to add the
// AnnotationOperatorVersion annotation with some value, which will then be overwritten by the web-hook
// and the Coherence resource will be correctly processes.
desiredResources = storage.GetLatest()
log.Info("Ignoring hash difference for pre-3.4.2 resource", "hash", hash, "store", storeHash)
}
}
} else {
// storage state was saved with the current hash so is already in the desired state
desiredResources = storage.GetLatest()
}
return desiredResources, err
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ go 1.23.0
toolchain go1.23.4

require (
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/distribution/reference v0.6.0
github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
github.com/go-logr/logr v1.4.2
Expand All @@ -31,6 +30,7 @@ require (
require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/emicklei/go-restful/v3 v3.12.1 // indirect
github.com/evanphx/json-patch v5.9.0+incompatible // indirect
github.com/evanphx/json-patch/v5 v5.9.0 // indirect
Expand Down
Loading