-
Notifications
You must be signed in to change notification settings - Fork 566
CNTRLPLANE-2834: add e2e test for etcd snapshot backup method on AWS #8231
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
openshift-merge-bot
merged 18 commits into
openshift:main
from
sdminonne:CNTRLPLANE-2834
Apr 27, 2026
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
d87d82f
test(CNTRLPLANE-2834): add e2e test for etcd snapshot backup method o…
sdminonne 5b12b57
test(CNTRLPLANE-2834): address review feedback for etcd snapshot e2e …
sdminonne 705db17
test(CNTRLPLANE-2834): address review feedback from mgencur and coder…
sdminonne 2cc25dd
test(CNTRLPLANE-2834): address review feedback from jparrill
sdminonne 142b11a
refactor(CNTRLPLANE-2834): extract shared helpers from backup/restore…
sdminonne 7191443
fix(CNTRLPLANE-2834): use else-if chain in etcd-init log parsing to a…
sdminonne b879134
fix(CNTRLPLANE-2834): require trailing hyphen in HCPEtcdBackup name m…
sdminonne 08dd8fa
fix(CNTRLPLANE-2834): skip dependent specs when snapshotURL is not ca…
sdminonne e3900c4
refactor(CNTRLPLANE-2834): use ring buffer for log tail and remove un…
sdminonne c170e7b
refactor(e2e): wire --use-etcd-snapshot flag to OADP e2e option structs
mgencur 99959c7
fix(CNTRLPLANE-2834): compare restoreSnapshotURL by presence instead …
sdminonne 3e46b7c
fix(CNTRLPLANE-2834): poll for workload readiness in pre-backup valid…
sdminonne 65bd426
fix(CNTRLPLANE-2834): poll for workload readiness in post-backup vali…
sdminonne b0a7d0a
fix(CNTRLPLANE-2834): ensure DPA has hypershift plugin before etcd sn…
sdminonne 4ca1390
fix(CNTRLPLANE-2834): wait for DPA reconciliation after adding hypers…
sdminonne 62ea67a
fix(CNTRLPLANE-2834): use deferred polling to avoid stale DPA status …
sdminonne 78db339
fix(CNTRLPLANE-2834): add diagnostic logging and increase DPA reconci…
sdminonne d8fde66
fix(CNTRLPLANE-2834): check customPlugins before adding hypershift to…
sdminonne File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| //go:build e2ev2 && backuprestore | ||
|
|
||
| package backuprestore | ||
|
|
||
| import ( | ||
| "bufio" | ||
| "context" | ||
| "fmt" | ||
| "io" | ||
| "strings" | ||
|
|
||
| "github.com/go-logr/logr" | ||
| hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" | ||
| "github.com/openshift/hypershift/test/e2e/v2/internal" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/api/meta" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/util/wait" | ||
| "k8s.io/client-go/kubernetes" | ||
| crclient "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| const ( | ||
| // EtcdPodName is the name of the etcd pod whose init container logs are verified. | ||
| EtcdPodName = "etcd-0" | ||
| // EtcdInitContainerName is the name of the init container in the etcd pod. | ||
| EtcdInitContainerName = "etcd-init" | ||
|
|
||
| // HCPEtcdBackupNamePrefix is the prefix used by the OADP plugin when creating | ||
| // HCPEtcdBackup resources. The full name follows the pattern: oadp-<BackupName>-<random>. | ||
| HCPEtcdBackupNamePrefix = "oadp-" | ||
|
|
||
| // logRestoringSnapshot is emitted by etcdutl/etcdctl when starting a snapshot restore. | ||
| logRestoringSnapshot = "restoring snapshot" | ||
| // logRestoredSnapshot is emitted by etcdutl/etcdctl when snapshot restore completes. | ||
| logRestoredSnapshot = "restored snapshot" | ||
| // logNotRestoringSnapshot indicates the restore was skipped because data already existed. | ||
| logNotRestoringSnapshot = "not empty, not restoring snapshot" | ||
| ) | ||
|
|
||
| // MatchesHCPEtcdBackupName checks whether an HCPEtcdBackup resource name matches the | ||
| // expected naming pattern for a given OADP backup name. The OADP plugin creates | ||
| // HCPEtcdBackup resources with the naming pattern: oadp-<BackupName>-<random>. | ||
| func MatchesHCPEtcdBackupName(hcpEtcdBackupName, oadpBackupName string) bool { | ||
| return strings.HasPrefix(hcpEtcdBackupName, HCPEtcdBackupNamePrefix+oadpBackupName+"-") | ||
| } | ||
|
|
||
| // WaitForHCPEtcdBackupCondition waits for an HCPEtcdBackup resource matching the given | ||
| // OADP backup name to have a BackupCompleted condition with the specified status. | ||
| // HCPEtcdBackup names follow the pattern: oadp-<BackupName>-<random>. | ||
| func WaitForHCPEtcdBackupCondition(testCtx *internal.TestContext, backupName string, expectedStatus metav1.ConditionStatus) error { | ||
| return wait.PollUntilContextTimeout(testCtx.Context, PollInterval, BackupTimeout, true, func(ctx context.Context) (bool, error) { | ||
| hcpEtcdBackupList := &hyperv1.HCPEtcdBackupList{} | ||
| if err := testCtx.MgmtClient.List(ctx, hcpEtcdBackupList, crclient.InNamespace(testCtx.ControlPlaneNamespace)); err != nil { | ||
| return false, fmt.Errorf("failed to list HCPEtcdBackup resources: %w", err) | ||
| } | ||
|
|
||
| for _, backup := range hcpEtcdBackupList.Items { | ||
| if !MatchesHCPEtcdBackupName(backup.Name, backupName) { | ||
| continue | ||
| } | ||
| condition := meta.FindStatusCondition(backup.Status.Conditions, string(hyperv1.BackupCompleted)) | ||
| if condition == nil { | ||
| return false, nil | ||
| } | ||
| if condition.Status == expectedStatus { | ||
| return true, nil | ||
| } | ||
| // If the condition is explicitly False, the backup failed - stop polling. | ||
| if expectedStatus == metav1.ConditionTrue && condition.Status == metav1.ConditionFalse { | ||
| return false, fmt.Errorf("HCPEtcdBackup %s has BackupCompleted=False: reason=%s, message=%s", | ||
| backup.Name, condition.Reason, condition.Message) | ||
| } | ||
| return false, nil | ||
| } | ||
| return false, nil | ||
| }) | ||
| } | ||
|
|
||
| // VerifyEtcdInitLogs retrieves the etcd-init container logs from the etcd-0 pod in the | ||
| // control plane namespace and verifies that they contain expected snapshot restore traces. | ||
| // The expected log lines from etcdutl/etcdctl indicate a successful snapshot restore: | ||
| // - "restoring snapshot" (restore started) | ||
| // - "restored snapshot" (restore completed) | ||
| // | ||
| // It also checks that the restore was not skipped due to existing data: | ||
| // - "not empty, not restoring snapshot" must NOT be present | ||
| func VerifyEtcdInitLogs(ctx context.Context, logger logr.Logger, kubeClient kubernetes.Interface, controlPlaneNamespace string) error { | ||
| podLogOpts := &corev1.PodLogOptions{ | ||
| Container: EtcdInitContainerName, | ||
| } | ||
|
|
||
| req := kubeClient.CoreV1().Pods(controlPlaneNamespace).GetLogs(EtcdPodName, podLogOpts) | ||
| logStream, err := req.Stream(ctx) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to stream %s container logs from %s: %w", EtcdInitContainerName, EtcdPodName, err) | ||
| } | ||
| defer logStream.Close() | ||
|
|
||
| result, err := parseEtcdInitLogs(logStream) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| logger.Info("etcd-init container logs scanned", "lines", result.lineCount) | ||
|
|
||
| if result.restoreSkipped { | ||
| for _, line := range result.tailLines { | ||
| logger.V(1).Info("etcd-init tail", "log", line) | ||
| } | ||
| return fmt.Errorf("etcd-init logs contain '%s'; restore was skipped because data directory was not empty", logNotRestoringSnapshot) | ||
| } | ||
| if !result.restoreStarted { | ||
| for _, line := range result.tailLines { | ||
| logger.V(1).Info("etcd-init tail", "log", line) | ||
| } | ||
| return fmt.Errorf("etcd-init logs do not contain '%s'; snapshot restore may not have started", logRestoringSnapshot) | ||
| } | ||
| if !result.restoreCompleted { | ||
| for _, line := range result.tailLines { | ||
| logger.V(1).Info("etcd-init tail", "log", line) | ||
| } | ||
| return fmt.Errorf("etcd-init logs do not contain '%s'; snapshot restore may have failed", logRestoredSnapshot) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // etcdInitLogResult holds the results of parsing etcd-init container logs. | ||
| type etcdInitLogResult struct { | ||
| restoreStarted bool | ||
| restoreCompleted bool | ||
| restoreSkipped bool | ||
| lineCount int | ||
| tailLines []string | ||
| } | ||
|
|
||
| // parseEtcdInitLogs scans etcd-init container log output and checks for expected | ||
| // snapshot restore trace messages from etcdutl/etcdctl. | ||
| func parseEtcdInitLogs(reader io.Reader) (*etcdInitLogResult, error) { | ||
| const tailSize = 50 | ||
|
|
||
| result := &etcdInitLogResult{} | ||
|
|
||
| // Use a ring buffer so old strings become eligible for GC immediately | ||
| // instead of being retained by the underlying slice array. | ||
| ring := make([]string, tailSize) | ||
| ringIdx := 0 | ||
| ringLen := 0 | ||
|
|
||
| scanner := bufio.NewScanner(reader) | ||
| buf := make([]byte, 256*1024) | ||
| scanner.Buffer(buf, 512*1024) | ||
| for scanner.Scan() { | ||
| line := scanner.Text() | ||
| result.lineCount++ | ||
| ring[ringIdx] = line | ||
| ringIdx = (ringIdx + 1) % tailSize | ||
| if ringLen < tailSize { | ||
| ringLen++ | ||
| } | ||
| lower := strings.ToLower(line) | ||
| if strings.Contains(lower, logNotRestoringSnapshot) { | ||
| result.restoreSkipped = true | ||
| } else if strings.Contains(lower, logRestoredSnapshot) { | ||
| result.restoreCompleted = true | ||
| } else if strings.Contains(lower, logRestoringSnapshot) { | ||
| result.restoreStarted = true | ||
| } | ||
| } | ||
| if err := scanner.Err(); err != nil { | ||
| return nil, fmt.Errorf("error reading etcd-init logs: %w", err) | ||
| } | ||
|
|
||
| // Flatten the ring buffer into chronological order. | ||
| result.tailLines = make([]string, ringLen) | ||
| start := (ringIdx - ringLen + tailSize) % tailSize | ||
| for i := range ringLen { | ||
| result.tailLines[i] = ring[(start+i)%tailSize] | ||
| } | ||
|
|
||
| return result, nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| //go:build e2ev2 && backuprestore | ||
|
|
||
| package backuprestore | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| . "github.com/onsi/gomega" | ||
| ) | ||
|
|
||
| func TestParseEtcdInitLogs(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| logs string | ||
| restoreStarted bool | ||
| restoreCompleted bool | ||
| restoreSkipped bool | ||
| lineCount int | ||
| }{ | ||
| { | ||
| name: "When etcd-init logs show successful snapshot restore it should detect both restoring and restored", | ||
| logs: `INFO: using etcdutl (etcd 3.6+) | ||
| +----------+----------+------------+------------+---------+ | ||
| | HASH | REVISION | TOTAL KEYS | TOTAL SIZE | VERSION | | ||
| +----------+----------+------------+------------+---------+ | ||
| | 5643d825 | 578454 | 3209 | 49 MB | 3.6.0 | | ||
| +----------+----------+------------+------------+---------+ | ||
| 2026-04-13T07:33:20Z info snapshot/v3_snapshot.go:305 restoring snapshot {"path": "/tmp/snapshot"} | ||
| 2026-04-13T07:33:20Z info membership/cluster.go:424 added member | ||
| 2026-04-13T07:33:20Z info snapshot/v3_snapshot.go:333 restored snapshot {"path": "/tmp/snapshot"}`, | ||
| restoreStarted: true, | ||
| restoreCompleted: true, | ||
| restoreSkipped: false, | ||
| lineCount: 9, | ||
| }, | ||
| { | ||
| name: "When data directory is not empty it should detect restore was skipped", | ||
| logs: `/var/lib/data not empty, not restoring snapshot`, | ||
| restoreStarted: false, | ||
| restoreCompleted: false, | ||
| restoreSkipped: true, | ||
| lineCount: 1, | ||
| }, | ||
| { | ||
| name: "When logs contain only curl progress it should detect neither restore started nor completed", | ||
| logs: ` % Total % Received % Xferd Average Speed Time Time Time Current`, | ||
| restoreStarted: false, | ||
| restoreCompleted: false, | ||
| restoreSkipped: false, | ||
| lineCount: 1, | ||
| }, | ||
| { | ||
| name: "When restore starts but does not complete it should detect only restoring", | ||
| logs: `INFO: using etcdutl (etcd 3.6+) | ||
| 2026-04-13T07:33:20Z info snapshot/v3_snapshot.go:305 restoring snapshot {"path": "/tmp/snapshot"}`, | ||
| restoreStarted: true, | ||
| restoreCompleted: false, | ||
| restoreSkipped: false, | ||
| lineCount: 2, | ||
| }, | ||
| { | ||
| name: "When logs are empty it should detect nothing", | ||
| logs: ``, | ||
| restoreStarted: false, | ||
| restoreCompleted: false, | ||
| restoreSkipped: false, | ||
| lineCount: 0, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| g := NewWithT(t) | ||
| reader := strings.NewReader(tt.logs) | ||
| result, err := parseEtcdInitLogs(reader) | ||
| g.Expect(err).NotTo(HaveOccurred()) | ||
| g.Expect(result.restoreStarted).To(Equal(tt.restoreStarted), "restoreStarted mismatch") | ||
| g.Expect(result.restoreCompleted).To(Equal(tt.restoreCompleted), "restoreCompleted mismatch") | ||
| g.Expect(result.restoreSkipped).To(Equal(tt.restoreSkipped), "restoreSkipped mismatch") | ||
| g.Expect(result.lineCount).To(Equal(tt.lineCount), "lineCount mismatch") | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestMatchesHCPEtcdBackupName(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| hcpEtcdBackupName string | ||
| oadpBackupName string | ||
| expectedMatch bool | ||
| }{ | ||
| { | ||
| name: "When HCPEtcdBackup name matches the oadp pattern it should return true", | ||
| hcpEtcdBackupName: "oadp-mycluster-mynamespace-abc123-xyz78", | ||
| oadpBackupName: "mycluster-mynamespace-abc123", | ||
| expectedMatch: true, | ||
| }, | ||
| { | ||
| name: "When HCPEtcdBackup name does not match it should return false", | ||
| hcpEtcdBackupName: "some-other-backup", | ||
| oadpBackupName: "mycluster-mynamespace-abc123", | ||
| expectedMatch: false, | ||
| }, | ||
| { | ||
| name: "When HCPEtcdBackup name is the exact backup name without prefix it should return false", | ||
| hcpEtcdBackupName: "mycluster-mynamespace-abc123", | ||
| oadpBackupName: "mycluster-mynamespace-abc123", | ||
| expectedMatch: false, | ||
| }, | ||
| { | ||
| name: "When HCPEtcdBackup name only shares a backup name prefix it should return false", | ||
| hcpEtcdBackupName: "oadp-mycluster-mynamespace-abc1234-xyz78", | ||
| oadpBackupName: "mycluster-mynamespace-abc123", | ||
| expectedMatch: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| g := NewWithT(t) | ||
| g.Expect(MatchesHCPEtcdBackupName(tt.hcpEtcdBackupName, tt.oadpBackupName)).To(Equal(tt.expectedMatch)) | ||
| }) | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.