fix(upgrade): handle mutated images by comparing the extracted version

Signed-off-by: Khalid Nowaf <f5f@msn.com>
longhorn · Jan 23, 2025 · 731735f · 731735f
1 parent 2f45864
commit 731735f
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 42 deletions.
diff --git a/upgrade/upgrade.go b/upgrade/upgrade.go
@@ -4,7 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"regexp"
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -353,19 +353,18 @@ func waitForOldLonghornManagersToBeFullyRemoved(namespace, managerImage string,
 }
 
 func isOldManagerPod(pod corev1.Pod, managerImage string) (bool, string) {
+	runningPodVersion, exists := pod.GetLabels()["app.kubernetes.io/version"]
+
 	for _, container := range pod.Spec.Containers {
 		if container.Name == "longhorn-manager" {
-			if extractSemver(container.Image) != extractSemver(managerImage) {
+			// if the pod has a version label and the label does not match
+			// the image version, then the running pod (Manger) is old
+			// we are doing this since images can be mutated by webhooks
+			if exists && !strings.Contains(managerImage, runningPodVersion) || runningPodVersion == "" {
 				return true, container.Image
 			}
 		}
 	}
-	return false, ""
-}
 
-func extractSemver(input string) string {
-	regex := regexp.MustCompile(`v\d+\.\d+\.\d+`)
-	match := regex.FindString(input)
-
-	return match
+	return false, ""
 }
diff --git a/upgrade/upgrage_test.go b/upgrade/upgrage_test.go
@@ -6,15 +6,20 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func newDummyPod(name string, image string) *corev1.Pod {
+func newDummyPod(name string, version string) *corev1.Pod {
 	return &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Labels: map[string]string{
+				"app.kubernetes.io/version": version,
+			},
+		},
 		Spec: corev1.PodSpec{
 			Containers: []corev1.Container{
 				{
-					Name:  name,
-					Image: image,
+					Name: name,
 				},
 			},
 		},
@@ -23,55 +28,54 @@ func newDummyPod(name string, image string) *corev1.Pod {
 
 func TestIsOldManagerPod(t *testing.T) {
 	tests := []struct {
-		runningManagerImage string
-		deployManagerImage  string
-		isOld               bool
+		runningPodVersionLabel string
+		deployingManagerImage  string
+		isOld                  bool
 	}{
 		{
-			runningManagerImage: "127.0.0.1:31999/longhornio/longhorn-manager:v1.8.0-zarf-4048341149",
-			deployManagerImage:  "longhornio/longhorn-manager:v1.8.0",
-			isOld:               false,
+			runningPodVersionLabel: "v1.8.0",
+			deployingManagerImage:  "longhornio/longhorn-manager:v1.8.0",
+			isOld:                  false,
 		},
 		{
-			runningManagerImage: "some.local.oci:5000/longhornio/longhorn-manager:v1.8.0-xyz423434.4.5.6.4-3-5",
-			deployManagerImage:  "longhornio/longhorn-manager:v1.8.0",
-			isOld:               false,
+			runningPodVersionLabel: "v1.8.0",
+			deployingManagerImage:  "some.local.oci:5000/longhornio/longhorn-manager:v1.8.0",
+			isOld:                  false,
 		},
-
 		{
-			runningManagerImage: "longhornio/longhorn-manager:v1.8.0",
-			deployManagerImage:  "longhornio/longhorn-manager:v1.8.0",
-			isOld:               false,
+			runningPodVersionLabel: "v1.8.0",
+			deployingManagerImage:  "longhornio/longhorn-manager:v1.8.0-patched4.5.6.4",
+			isOld:                  false,
 		},
 		{
-			runningManagerImage: "longhornio/longhorn-manager:v1.8.0",
-			deployManagerImage:  "longhornio/longhorn-manager:v1.9.0",
-			isOld:               true,
+			runningPodVersionLabel: "v1.8.0-dev-20250112",
+			deployingManagerImage:  "longhornio/longhorn-manager:-v1.8.0-dev-20250112",
+			isOld:                  false,
 		},
 		{
-			runningManagerImage: "some.local.oci:5000/longhornio/longhorn-manager:v1.8.0-xyz423434.4.5.6.4",
-			deployManagerImage:  "longhornio/longhorn-manager:v1.9.0",
-			isOld:               true,
+			runningPodVersionLabel: "v1.8.0",
+			deployingManagerImage:  "longhornio/longhorn-manager:v1.9.0",
+			isOld:                  true,
 		},
 		{
-			runningManagerImage: "some.local.oci:5000/longhornio/longhorn-manager:v1.8.0-xyz423434.4.5.6.4",
-			deployManagerImage:  "longhornio/longhorn-manager:some-non-semver",
-			isOld:               true,
+			runningPodVersionLabel: "",
+			deployingManagerImage:  "some.local.oci:5000/longhornio/longhorn-manager:v1.8.0-dev-20250112",
+			isOld:                  true,
 		},
 		{
-			runningManagerImage: "some.local.oci:5000/longhornio/longhorn-manager:some-non-semver",
-			deployManagerImage:  "longhornio/longhorn-manager:some-non-semver-xx.xx.xx",
-			isOld:               false,
+			runningPodVersionLabel: "",
+			deployingManagerImage:  "longhornio/longhorn-manager:some-non-semver",
+			isOld:                  true,
 		},
 	}
 
 	for _, tt := range tests {
-		runningPods := newDummyPod("longhorn-manager", tt.runningManagerImage)
-		isOld, _ := isOldManagerPod(*runningPods, tt.deployManagerImage)
+		runningPods := newDummyPod("longhorn-manager", tt.runningPodVersionLabel)
+		isOld, _ := isOldManagerPod(*runningPods, tt.deployingManagerImage)
+
 		assert.Equal(t, tt.isOld, isOld,
-			fmt.Sprintf("compering both images version \n	-%s (%s)\n	-%s (%s)",
-				tt.runningManagerImage, extractSemver(tt.runningManagerImage),
-				tt.deployManagerImage, extractSemver(tt.deployManagerImage),
+			fmt.Sprintf("compering both Label: %s to Image %s",
+				tt.runningPodVersionLabel, tt.deployingManagerImage,
 			),
 		)
 	}