Fix version in status for OpenTelemetry Collector after upgrades (#1803)

* Fix version in status for OpenTelemetry Collector after upgrades #1802 Signed-off-by: Israel Blancas <[email protected]> * Add missing changelog Signed-off-by: Israel Blancas <[email protected]> * Fix unit tests Signed-off-by: Israel Blancas <[email protected]> * Add unit test to cover this use case Signed-off-by: Israel Blancas <[email protected]> * Add new log message Signed-off-by: Israel Blancas <[email protected]> --------- Signed-off-by: Israel Blancas <[email protected]>
open-telemetry · Jun 8, 2023 · d6ed973 · d6ed973
1 parent c257a7e
commit d6ed973
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 20 deletions.
diff --git a/.chloggen/collector-shown-version-is-incorrect-after-upgrade.yaml b/.chloggen/collector-shown-version-is-incorrect-after-upgrade.yaml
@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
+component: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: The OpenTelemetry Collector version is not shown properly in the status field if no upgrade routines are performed.
+
+# One or more tracking issues related to the change
+issues: [1802]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
diff --git a/pkg/collector/upgrade/upgrade.go b/pkg/collector/upgrade/upgrade.go
@@ -108,7 +108,20 @@ func (u VersionUpgrade) ManagedInstance(ctx context.Context, otelcol v1alpha1.Op
 	}
 
 	if instanceV.GreaterThan(&Latest.Version) {
-		u.Log.Info("skipping upgrade for OpenTelemetry Collector instance, as it's newer than our latest version", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version, "latest", Latest.Version.String())
+		// Update with the latest known version, which is what we have from versions.txt
+		u.Log.Info("no upgrade routines are needed for the OpenTelemetry instance", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version, "latest", Latest.Version.String())
+
+		otelColV, err := semver.NewVersion(u.Version.OpenTelemetryCollector)
+		if err != nil {
+			return otelcol, err
+		}
+		if instanceV.LessThan(otelColV) {
+			u.Log.Info("upgraded OpenTelemetry Collector version", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version)
+			otelcol.Status.Version = u.Version.OpenTelemetryCollector
+		} else {
+			u.Log.Info("skipping upgrade for OpenTelemetry Collector instance", "name", otelcol.Name, "namespace", otelcol.Namespace)
+		}
+
 		return otelcol, nil
 	}
 
@@ -126,8 +139,7 @@ func (u VersionUpgrade) ManagedInstance(ctx context.Context, otelcol v1alpha1.Op
 			otelcol = *upgraded
 		}
 	}
-
-	// at the end of the process, we are up to date with the latest known version, which is what we have from versions.txt
+	// Update with the latest known version, which is what we have from versions.txt
 	otelcol.Status.Version = u.Version.OpenTelemetryCollector
 
 	u.Log.V(1).Info("final version", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version)

diff --git a/pkg/collector/upgrade/upgrade_test.go b/pkg/collector/upgrade/upgrade_test.go
@@ -84,25 +84,36 @@ func TestShouldUpgradeAllToLatestBasedOnUpgradeStrategy(t *testing.T) {
 }
 
 func TestUpgradeUpToLatestKnownVersion(t *testing.T) {
-	// prepare
-	nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
-	existing := makeOtelcol(nsn)
-	existing.Status.Version = "0.8.0"
+	for _, tt := range []struct {
+		desc      string
+		v         string
+		expectedV string
+	}{
+		{"upgrade-routine", "0.8.0", "0.10.0"},     // we don't have a 0.10.0 upgrade, but we have a 0.9.0
+		{"no-upgrade-routine", "0.61.1", "0.62.0"}, // No upgrade routines between these two versions
+	} {
+		t.Run(tt.desc, func(t *testing.T) {
+			// prepare
+			nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
+			existing := makeOtelcol(nsn)
+			existing.Status.Version = tt.v
 
-	currentV := version.Get()
-	currentV.OpenTelemetryCollector = "0.10.0" // we don't have a 0.10.0 upgrade, but we have a 0.9.0
-	up := &upgrade.VersionUpgrade{
-		Log:      logger,
-		Version:  currentV,
-		Client:   k8sClient,
-		Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize),
-	}
-	// test
-	res, err := up.ManagedInstance(context.Background(), existing)
+			currentV := version.Get()
+			currentV.OpenTelemetryCollector = tt.expectedV
+			up := &upgrade.VersionUpgrade{
+				Log:      logger,
+				Version:  currentV,
+				Client:   k8sClient,
+				Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize),
+			}
+			// test
+			res, err := up.ManagedInstance(context.Background(), existing)
 
-	// verify
-	assert.NoError(t, err)
-	assert.Equal(t, "0.10.0", res.Status.Version)
+			// verify
+			assert.NoError(t, err)
+			assert.Equal(t, tt.expectedV, res.Status.Version)
+		})
+	}
 }
 
 func TestVersionsShouldNotBeChanged(t *testing.T) {