Skip to content

OCPNODE-4055: Add comprehensive test suite for Additional Storage Support feature #31083

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.

Sign up for GitHub

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

Closed
BhargaviGudi wants to merge 1 commit into from
Closed

OCPNODE-4055: Add comprehensive test suite for Additional Storage Support feature #31083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 81 additions & 23 deletions test/extended/node/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,46 +13,77 @@ This directory contains OpenShift end-to-end tests for node-related features.
- **node_e2e/netns_cleanup.go** - Network namespace cleanup - Verifies kubelet/CRI-O properly deletes network namespace when a pod is deleted \[OTP\]
- **node_e2e/pdb_drain.go** - PodDisruptionBudget drain blocking (OCP-67564) - Tests that node drain is blocked when PDB has minAvailable=100% with empty selector \[Disruptive\] \[Lifecycle:informing\]

### Suite: openshift/conformance/parallel

- **Additional Storage Support API Validation** - API validation tests for additionalArtifactStores, additionalImageStores, and additionalLayerStores CRI-O configuration
- **Additional Storage Support** - Tests for additionalArtifactStores, additionalImageStores, and additionalLayerStores CRI-O configuration

**Availability:**
- **OCP 4.22:** TechPreview (requires TechPreviewNoUpgrade feature gate)
- **OCP 4.23+/5.0+:** GA (Generally Available)

**Suite:** `openshift/conformance/parallel`
**Suite:** `openshift/conformance/parallel` (API tests), `openshift/disruptive-longrunning` (E2E tests)
**Feature Tag:** `[Feature:AdditionalStorageSupport]`
**Sig Tag:** `[sig-node]`
**OCPFeatureGate:** `AdditionalStorageConfig`
**Sig Tag:** `[sig-node]`

**Test File:**
**Test Files:**
- **additional_storage_api.go** - 13 API validation tests using DryRun (non-disruptive, parallel execution)
- Combined Additional Stores (3 tests): invalid paths, max count enforcement, duplicate detection
- Additional Layer Stores (8 tests): comprehensive path validation (empty, relative, spaces, special chars, length, max count, consecutive slashes, duplicates)
- Additional Image Stores (1 smoke test): path validation wiring
- Additional Artifact Stores (1 smoke test): path validation wiring
- **additional_storage_e2e.go** - 3 E2E tests with single-node MCP rollouts (disruptive, serial execution)
- Combined Stores - Configuration & Verification: validates all three storage types configure correctly
- Combined Stores - Functional Verification: tests layer/image/artifact stores functionality with prepopulated images and registry fallback
- Layer Stores - Comprehensive Lifecycle: stargz deployment, lazy pulling, store updates, maximum stores, CRC deletion cleanup, and fallback scenarios (standard OCI image, stopped stargz-store)
- **stargz_store_setup.go** - Helper for deploying/cleaning up stargz-store daemonset on worker nodes

**Requirements:**
- AdditionalStorageConfig feature gate must be enabled
- API tests are non-disruptive (use DryRun)
- Run in parallel with other conformance tests
- Skip on MicroShift (no MachineConfig support)
- Skip on Microsoft Azure (known platform issues)
- TechPreviewNoUpgrade feature gate must be enabled (only for OCP 4.22)
- API tests: Non-disruptive, use DryRun, run in parallel (requires `OCPFeatureGate:AdditionalStorageConfig`)
- E2E tests: Serial and Disruptive, use single-node MCP for faster rollouts (~10-15 min vs ~25 min)
- Tests that pull external images are tagged [Skipped:Disconnected] for air-gapped environments

**Test Environments:**

These tests run in CI on multiple platforms via `disruptive-longrunning-techpreview` jobs:
- AWS (primary platform for Additional Storage Support E2E tests)
- Azure (E2E tests explicitly skip Azure via platform detection)
- GCP
- vSphere
- Metal IPI (IPv6 and dual-stack)

**CI Job Configuration:**
- **Feature Set:** `TechPreviewNoUpgrade` (enables tech preview features)
- **Test Suite:** `openshift/disruptive-longrunning`
- **Interval:** Weekly (168h)
- **Sharding:** 2 shards to parallelize execution
- **Job definitions:** See `ci-operator/config/openshift/release/openshift-release-main__nightly-4.XX.yaml` in [openshift/release](https://github.com/openshift/release) repo

**Running API validation tests:**
**Running these tests:**
```bash
# Run only Additional Storage API validation tests (fast, non-disruptive)
./openshift-tests run "openshift/conformance/parallel" --dry-run | \
grep "\[Feature:AdditionalStorageSupport\]" | \
./openshift-tests run -f -
# Run API validation tests (fast, non-disruptive, parallel)
./openshift-tests run "openshift/conformance/parallel" --dry-run | grep "\[Feature:AdditionalStorageSupport\]" | ./openshift-tests run -f -

# Run E2E tests (slow, disruptive, triggers MCP rollouts)
./openshift-tests run "openshift/disruptive-longrunning" --dry-run | grep "\[Feature:AdditionalStorageSupport\]" | ./openshift-tests run -f - --cluster-stability=Disruptive

# Run all tests (API + E2E)
./openshift-tests run "openshift/conformance/parallel" --dry-run | grep "\[Feature:AdditionalStorageSupport\]" | ./openshift-tests run -f - && \
./openshift-tests run "openshift/disruptive-longrunning" --dry-run | grep "\[Feature:AdditionalStorageSupport\]" | ./openshift-tests run -f - --cluster-stability=Disruptive
```

**Test Coverage (13 tests, ~2-3 min total):**
- Path format validation (absolute paths, character restrictions, length limits)
- Count limits enforcement (5 for artifact/layer stores, 10 for image stores)
- Duplicate path detection within store types
- Combined store configurations with invalid paths
**Test Coverage:**
- **API Validation (13 tests, ~2-3 min total):**
- Path format validation (absolute paths, character restrictions, length limits)
- Count limits enforcement (5 for artifact/layer stores, 10 for image stores)
- Duplicate path detection within store types
- Combined store configurations with invalid paths
- **E2E Tests (3 tests, ~45-60 min total):**
- CRI-O storage.conf generation and verification on worker nodes
- MachineConfigOperator (MCO) configuration updates and single-node MCP rollouts
- Lazy pulling with eStargz images and stargz-store snapshotter
- Prepopulated image store functionality and registry fallback
- Artifact store read/write verification
- Layer store updates (2 stores, max 5 stores) and CRC deletion cleanup
- Fallback behavior: standard OCI images and stopped stargz-store service

### Suite: openshift/usernamespace

Expand All @@ -73,7 +104,9 @@ This directory contains OpenShift end-to-end tests for node-related features.
- Each file focuses on a specific node feature

### Utility Files
- **node_utils.go** - Shared helper functions for node selection and kubelet configuration retrieval
- **node_utils.go** - Shared helper functions for node selection, kubelet configuration retrieval, and Additional Storage Support skip/platform helpers
- **node_mcp_helpers.go** - MCP lifecycle helpers: single-node MachineConfigPool creation/cleanup, directory management on nodes, pod creation/deletion, and MCP/CRC wait utilities
- **stargz_store_setup.go** - Helper for deploying/cleaning up stargz-store daemonset on worker nodes for lazy pulling tests

### Test Data
Test fixtures are referenced via `exutil.FixturePath` from:
Expand Down Expand Up @@ -125,6 +158,31 @@ Useful links for `periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-disrup
- [Previous runs (Sippy)](https://sippy.dptools.openshift.org/sippy-ng/jobs/4.22/analysis?filters=%7B%22items%22%3A%5B%7B%22columnField%22%3A%22name%22%2C%22operatorValue%22%3A%22equals%22%2C%22value%22%3A%22periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-disruptive-longrunning%22%7D%5D%7D)
- [Job history for latest runs (Prow)](https://prow.ci.openshift.org/job-history/gs/test-platform-results/logs/periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-disruptive-longrunning)

### Adding TechPreview Tests to `openshift/disruptive-longrunning`

For tests that require TechPreviewNoUpgrade feature gate (like Additional Storage Support in OCP 4.22), use the TechPreview variant of the disruptive-longrunning CI job:

```
# For OCP 4.22 TechPreview testing
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-disruptive-longrunning-techpreview

# For OCP 4.23+ (GA), use standard job
/payload-job periodic-ci-openshift-release-main-nightly-4.23-e2e-aws-disruptive-longrunning
```

**Available platform variants:**
- `-e2e-aws-disruptive-longrunning-techpreview` (AWS - primary for storage tests)
- `-e2e-azure-disruptive-longrunning-techpreview` (Azure)
- `-e2e-gcp-disruptive-longrunning-techpreview` (GCP)
- `-e2e-vsphere-disruptive-longrunning-techpreview` (vSphere)
- `-e2e-metal-ipi-ovn-ipv6-disruptive-longrunning-techpreview` (Bare Metal IPv6)
- `-e2e-metal-ipi-ovn-dual-disruptive-longrunning-techpreview` (Bare Metal dual-stack)

**CI Job Configuration:**
- Job definitions: `ci-operator/config/openshift/release/openshift-release-main__nightly-4.XX.yaml` in [openshift/release](https://github.com/openshift/release) repo
- Feature Set: `TechPreviewNoUpgrade`
- Test Suite: `openshift/disruptive-longrunning`

## Important Notes

- Note that dry-run option won't list the test as it does not connect to a live cluster
Expand Down
26 changes: 15 additions & 11 deletions test/extended/node/additional_storage_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
)

// API validation tests - use DryRun to avoid triggering MCO reconciliation
// Run in disruptive-longrunning suite to ensure TechPreview feature gate is enabled
var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration.openshift.io][Jira:Node/CRI-O][sig-node][Feature:AdditionalStorageSupport][OCPFeatureGate:AdditionalStorageConfig][Suite:openshift/conformance/parallel] Additional Storage API Validation", func() {
defer g.GinkgoRecover()

Expand All @@ -31,7 +32,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
// ========================================================================
g.Context("Combined Additional Stores", func() {
// Reject if any store type has invalid path
g.It("should reject if any store type has invalid path in combined config ", func(ctx context.Context) {
g.It("should reject if any store type has invalid path in combined config", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -71,7 +72,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
})

// Reject if layer stores exceed max while other stores are valid
g.It("should reject if layer stores exceed max even with valid image/artifact stores ", func(ctx context.Context) {
g.It("should reject if layer stores exceed max even with valid image/artifact stores", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -112,7 +113,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
})

// Reject duplicate paths within same store type in combined config
g.It("should reject duplicate paths within same store type in combined config ", func(ctx context.Context) {
g.It("should reject duplicate paths within same store type in combined config", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -155,7 +156,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
g.Context("Additional Layer Stores", func() {
// Should fail if additionalLayerStores path is empty
// Note: Go API returns "Required value" while YAML returns "at least 1 chars long"
g.It("should reject empty path for additionalLayerStores ", func(ctx context.Context) {
g.It("should reject empty path for additionalLayerStores", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -189,7 +190,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
})

// Should fail if additionalLayerStores path is not absolute
g.It("should reject relative path for additionalLayerStores ", func(ctx context.Context) {
g.It("should reject relative path for additionalLayerStores", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -221,7 +222,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
})

// Should fail if additionalLayerStores path contains spaces
g.It("should reject path with spaces for additionalLayerStores ", func(ctx context.Context) {
g.It("should reject path with spaces for additionalLayerStores", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -253,7 +254,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
})

// Should fail if additionalLayerStores path contains invalid characters
g.It("should reject path with invalid characters for additionalLayerStores ", func(ctx context.Context) {
g.It("should reject path with invalid characters for additionalLayerStores", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand All @@ -280,11 +281,12 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
ctx, ctrcfg, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}},
)
o.Expect(err).To(o.HaveOccurred(), "Expected API to reject path with invalid character '@'")
Comment thread
BhargaviGudi marked this conversation as resolved.
o.Expect(err.Error()).To(o.ContainSubstring("path must be absolute and contain only alphanumeric characters"))
framework.Logf("Path with '@' correctly rejected: %v", err)
})

// Should fail if additionalLayerStores path is too long (>256 bytes)
g.It("should reject path exceeding 256 characters for additionalLayerStores ", func(ctx context.Context) {
g.It("should reject path exceeding 256 characters for additionalLayerStores", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -318,7 +320,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
})

// Should fail if additionalLayerStores exceeds maximum of 5 items
g.It("should reject more than 5 additionalLayerStores ", func(ctx context.Context) {
g.It("should reject more than 5 additionalLayerStores", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -353,7 +355,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
})

// Should fail if additionalLayerStores path contains consecutive forward slashes
g.It("should reject path with consecutive forward slashes for additionalLayerStores ", func(ctx context.Context) {
g.It("should reject path with consecutive forward slashes for additionalLayerStores", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -385,7 +387,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
})

// Should fail if additionalLayerStores contains duplicate paths
g.It("should reject duplicate paths in additionalLayerStores ", func(ctx context.Context) {
g.It("should reject duplicate paths in additionalLayerStores", func(ctx context.Context) {
mcClient, err := mcclient.NewForConfig(oc.KubeFramework().ClientConfig())
o.Expect(err).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -450,6 +452,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
ctx, ctrcfg, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}},
)
o.Expect(err).To(o.HaveOccurred(), "Expected validation to reject invalid path")
o.Expect(err.Error()).To(o.ContainSubstring("path must be absolute and contain only alphanumeric characters"))
framework.Logf("Smoke test PASSED: Validation correctly rejected invalid path: %v", err)
})
})
Expand Down Expand Up @@ -486,6 +489,7 @@ var _ = g.Describe("[apigroup:config.openshift.io][apigroup:machineconfiguration
ctx, ctrcfg, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}},
)
o.Expect(err).To(o.HaveOccurred(), "Expected validation to reject invalid path")
o.Expect(err.Error()).To(o.ContainSubstring("path must be absolute and contain only alphanumeric characters"))
framework.Logf("Smoke test PASSED: Validation correctly rejected invalid path: %v", err)
})
})
Expand Down
Loading