Skip to content

Commit 9174a01

Browse files
chore(ci): Add test to check if user changes are preserved
Add a test to ensure that OLM is not reverting user changes like kubectl rollout restart. Assisted-by: Cursor/Claude
1 parent 95afce0 commit 9174a01

2 files changed

Lines changed: 120 additions & 0 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
Feature: Rollout Restart User Changes
2+
# Verifies that user-added pod template annotations persist after OLM reconciliation.
3+
# Fixes: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
4+
5+
Background:
6+
Given OLM is available
7+
And ClusterCatalog "test" serves bundles
8+
And ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
9+
10+
Scenario: User-initiated deployment changes persist after OLM reconciliation
11+
When ClusterExtension is applied
12+
"""
13+
apiVersion: olm.operatorframework.io/v1
14+
kind: ClusterExtension
15+
metadata:
16+
name: ${NAME}
17+
spec:
18+
namespace: ${TEST_NAMESPACE}
19+
serviceAccount:
20+
name: olm-sa
21+
source:
22+
sourceType: Catalog
23+
catalog:
24+
packageName: test
25+
selector:
26+
matchLabels:
27+
"olm.operatorframework.io/metadata.name": test-catalog
28+
"""
29+
Then ClusterExtension is available
30+
And resource "deployment/test-operator" is available
31+
When user performs rollout restart on "deployment/test-operator"
32+
Then deployment "test-operator" has restart annotation
33+
# Wait for at least two OLM reconciliation cycles (controller runs every 10s)
34+
And I wait for "30" seconds
35+
# Verify user changes persisted after OLM reconciliation
36+
Then deployment "test-operator" has restart annotation

test/e2e/steps/steps.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"os/exec"
1515
"path/filepath"
1616
"reflect"
17+
"strconv"
1718
"strings"
1819
"time"
1920

@@ -90,6 +91,9 @@ func RegisterSteps(sc *godog.ScenarioContext) {
9091
sc.Step(`^(?i)resource apply fails with error msg containing "([^"]+)"$`, ResourceApplyFails)
9192
sc.Step(`^(?i)resource "([^"]+)" is eventually restored$`, ResourceRestored)
9293
sc.Step(`^(?i)resource "([^"]+)" matches$`, ResourceMatches)
94+
sc.Step(`^(?i)user performs rollout restart on "([^"]+)"$`, UserPerformsRolloutRestart)
95+
sc.Step(`^(?i)deployment "([^"]+)" has restart annotation$`, DeploymentHasRestartAnnotation)
96+
sc.Step(`^(?i)I wait for "([^"]+)" seconds$`, WaitForSeconds)
9397

9498
sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in test namespace$`, ServiceAccountWithNeededPermissionsIsAvailableInNamespace)
9599
sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in \${TEST_NAMESPACE}$`, ServiceAccountWithNeededPermissionsIsAvailableInNamespace)
@@ -1303,3 +1307,83 @@ func latestActiveRevisionForExtension(extName string) (*ocv1.ClusterExtensionRev
13031307

13041308
return latest, nil
13051309
}
1310+
1311+
// UserPerformsRolloutRestart simulates a user running "kubectl rollout restart deployment/<name>".
1312+
// This adds a restart annotation to trigger a rolling restart of pods.
1313+
// This is used to test the generic fix - OLM should not undo ANY user-added annotations.
1314+
// In OLMv0, OLM would undo this change. In OLMv1, it should stay because kubectl owns it.
1315+
// See: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
1316+
func UserPerformsRolloutRestart(ctx context.Context, resourceName string) error {
1317+
sc := scenarioCtx(ctx)
1318+
resourceName = substituteScenarioVars(resourceName, sc)
1319+
1320+
kind, deploymentName, ok := strings.Cut(resourceName, "/")
1321+
if !ok {
1322+
return fmt.Errorf("invalid resource name format: %s (expected kind/name)", resourceName)
1323+
}
1324+
1325+
if kind != "deployment" {
1326+
return fmt.Errorf("only deployment resources are supported for restart annotation, got: %s", kind)
1327+
}
1328+
1329+
// Run kubectl rollout restart to add the restart annotation.
1330+
// This is the real command users run, so we test actual user behavior.
1331+
out, err := k8sClient("rollout", "restart", resourceName, "-n", sc.namespace)
1332+
if err != nil {
1333+
return fmt.Errorf("failed to rollout restart %s: %w; stderr: %s", resourceName, err, stderrOutput(err))
1334+
}
1335+
1336+
logger.V(1).Info("Rollout restart initiated", "deployment", deploymentName, "output", out)
1337+
1338+
return nil
1339+
}
1340+
1341+
// DeploymentHasRestartAnnotation checks that a deployment's pod template has
1342+
// the kubectl.kubernetes.io/restartedAt annotation. Fails immediately if absent,
1343+
// so a failing boxcutter scenario won't stall the entire suite.
1344+
func DeploymentHasRestartAnnotation(ctx context.Context, deploymentName string) error {
1345+
sc := scenarioCtx(ctx)
1346+
deploymentName = substituteScenarioVars(deploymentName, sc)
1347+
1348+
restartAnnotationKey := "kubectl.kubernetes.io/restartedAt"
1349+
out, err := k8sClient("get", "deployment", deploymentName, "-n", sc.namespace,
1350+
"-o", fmt.Sprintf("jsonpath={.spec.template.metadata.annotations['%s']}", restartAnnotationKey))
1351+
if err != nil {
1352+
return fmt.Errorf("failed to get restart annotation on deployment %s: %w; stderr: %s", deploymentName, err, stderrOutput(err))
1353+
}
1354+
1355+
if strings.TrimSpace(out) == "" {
1356+
return fmt.Errorf("deployment %s is missing expected annotation %s", deploymentName, restartAnnotationKey)
1357+
}
1358+
1359+
logger.V(1).Info("Restart annotation found", "deployment", deploymentName, "restartedAt", strings.TrimSpace(out))
1360+
return nil
1361+
}
1362+
1363+
// WaitForSeconds waits for the given number of seconds.
1364+
// Used when a test needs to ensure that at least one OLM reconciliation cycle
1365+
// has occurred before checking a condition. Since the controllers are event-driven,
1366+
// we use a generous fixed delay rather than polling for an observable signal.
1367+
func WaitForSeconds(ctx context.Context, seconds string) error {
1368+
sec, err := strconv.Atoi(seconds)
1369+
if err != nil {
1370+
return fmt.Errorf("invalid seconds value %s: %w", seconds, err)
1371+
}
1372+
1373+
if sec <= 0 {
1374+
return fmt.Errorf("seconds value must be greater than 0, got %d", sec)
1375+
}
1376+
1377+
logger.V(1).Info("Waiting for reconciliation", "seconds", sec)
1378+
1379+
timer := time.NewTimer(time.Duration(sec) * time.Second)
1380+
defer timer.Stop()
1381+
1382+
select {
1383+
case <-timer.C:
1384+
logger.V(1).Info("Wait complete")
1385+
return nil
1386+
case <-ctx.Done():
1387+
return fmt.Errorf("wait canceled: %w", ctx.Err())
1388+
}
1389+
}

0 commit comments

Comments
 (0)