From 2b3655ecb117beb14bf6dca62b2610fb3ee33283 Mon Sep 17 00:00:00 2001 From: Yuan Tang Date: Wed, 31 Mar 2021 22:04:47 -0400 Subject: [PATCH] fix: Remove invalid label value for last hit timestamp from caches (#5528) Signed-off-by: terrytangyuan --- workflow/common/common.go | 3 --- workflow/controller/cache/configmap_cache.go | 3 --- workflow/controller/cache_test.go | 15 ++------------- 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/workflow/common/common.go b/workflow/common/common.go index d35119182e53..b2f5a2fff46f 100644 --- a/workflow/common/common.go +++ b/workflow/common/common.go @@ -84,9 +84,6 @@ const ( // LabelKeyOnExit is a label applied to Pods that are run from onExit nodes, so that they are not shut down when stopping a Workflow LabelKeyOnExit = workflow.WorkflowFullName + "/on-exit" - // LabelKeyCacheLastHitTimestamp is the timestamp when the memoization cache is last hit. - LabelKeyCacheLastHitTimestamp = "last-hit-timestamp" - // ExecutorArtifactBaseDir is the base directory in the init container in which artifacts will be copied to. // Each artifact will be named according to its input name (e.g: /argo/inputs/artifacts/CODE) ExecutorArtifactBaseDir = "/argo/inputs/artifacts" diff --git a/workflow/controller/cache/configmap_cache.go b/workflow/controller/cache/configmap_cache.go index 421470bac826..d272f5b08d6e 100644 --- a/workflow/controller/cache/configmap_cache.go +++ b/workflow/controller/cache/configmap_cache.go @@ -15,7 +15,6 @@ import ( "k8s.io/client-go/kubernetes" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" - "github.com/argoproj/argo-workflows/v3/workflow/common" ) type configMapCache struct { @@ -62,7 +61,6 @@ func (c *configMapCache) Load(ctx context.Context, key string) (*Entry, error) { c.logInfo(log.Fields{}, "config map cache loaded") hitTime := time.Now() - cm.SetLabels(map[string]string{common.LabelKeyCacheLastHitTimestamp: hitTime.Format(time.RFC3339)}) rawEntry, ok := cm.Data[key] if !ok || rawEntry == "" { c.logInfo(log.Fields{}, "config map cache miss: entry does not exist") @@ -119,7 +117,6 @@ func (c *configMapCache) Save(ctx context.Context, key string, nodeId string, va } creationTime := time.Now() - cache.SetLabels(map[string]string{common.LabelKeyCacheLastHitTimestamp: creationTime.Format(time.RFC3339)}) newEntry := Entry{ NodeID: nodeId, diff --git a/workflow/controller/cache_test.go b/workflow/controller/cache_test.go index bbd3e51db6fb..1358430aa815 100644 --- a/workflow/controller/cache_test.go +++ b/workflow/controller/cache_test.go @@ -4,14 +4,12 @@ import ( "context" "encoding/json" "testing" - "time" "github.com/stretchr/testify/assert" apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" - "github.com/argoproj/argo-workflows/v3/workflow/common" "github.com/argoproj/argo-workflows/v3/workflow/controller/cache" ) @@ -55,13 +53,7 @@ func TestConfigMapCacheLoadHit(t *testing.T) { entry, err := c.Load(ctx, "hi-there-world") assert.NoError(t, err) - - cm, err = controller.kubeclientset.CoreV1().ConfigMaps("default").Get(ctx, sampleConfigMapCacheEntry.Name, metav1.GetOptions{}) - assert.NoError(t, err) - lastHitTimestampLabel, err := time.Parse(time.RFC3339, cm.Labels[common.LabelKeyCacheLastHitTimestamp]) - assert.NoError(t, err) - assert.True(t, lastHitTimestampLabel.After(entry.CreationTimestamp.Time)) - assert.Equal(t, lastHitTimestampLabel.Format(time.RFC3339), entry.LastHitTimestamp.Time.Format(time.RFC3339)) + assert.True(t, entry.LastHitTimestamp.Time.After(entry.CreationTimestamp.Time)) outputs := entry.Outputs assert.NoError(t, err) @@ -103,11 +95,8 @@ func TestConfigMapCacheSave(t *testing.T) { cm, err := controller.kubeclientset.CoreV1().ConfigMaps("default").Get(ctx, "whalesay-cache", metav1.GetOptions{}) assert.NoError(t, err) assert.NotNil(t, cm) - lastHitTimestampLabel, err := time.Parse(time.RFC3339, cm.Labels[common.LabelKeyCacheLastHitTimestamp]) - assert.NoError(t, err) var entry cache.Entry err = json.Unmarshal([]byte(cm.Data["hi-there-world"]), &entry) assert.NoError(t, err) - assert.Equal(t, lastHitTimestampLabel.Format(time.RFC3339), entry.LastHitTimestamp.Time.Format(time.RFC3339)) - assert.Equal(t, lastHitTimestampLabel.Format(time.RFC3339), entry.CreationTimestamp.Time.Format(time.RFC3339)) + assert.Equal(t, entry.LastHitTimestamp.Time, entry.CreationTimestamp.Time) }