-
Notifications
You must be signed in to change notification settings - Fork 88
operator: add ephemeral volume support for Clair (PROJQUAY-6684) #1059
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've reviewed this pull request using the Sourcery rules engine
Signed-off-by: dmesser <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces optional ephemeral volume support for the Clair container, enabling a generic ephemeral volume to be mounted at /tmp for the managed Clair component.
- Adds a new boolean override (useEphemeralVolume) along with storageClassName and volumeSize options for Clair.
- Injects the ephemeral volume into the Clair deployment and updates status reporting and validation logic.
- Enhances unit and integration tests and updates CRD definitions accordingly.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/middleware/middleware.go | Adds constants for deployment suffixes and injects ephemeral volume. |
| pkg/cmpstatus/clair_test.go | Provides extended test cases to validate ephemeral volume provisioning. |
| pkg/cmpstatus/clair.go | Adds PVC status checks for ephemeral volume with provisioning failure events. |
| e2e/storageclass_overrides/00-create-quay-registry.yaml | Minor update removing an extra configBundleSecret entry. |
| e2e/ephemeralvolume_overrides/00-create-quay-registry.yaml | Adds a new test CR for ephemeral volume overrides. |
| e2e/ephemeralvolume_overrides/00-assert.yaml | Asserts the presence and configuration of ephemeral volume injection. |
| config/crd/bases/quay.redhat.com_quayregistries.yaml | Updates CRD schema to include the useEphemeralVolume override. |
| bundle/manifests/quayregistries.crd.yaml | Mirrors CRD changes in the bundle manifest. |
| apis/quay/v1/zz_generated.deepcopy.go | Updates deepcopy functions for the new UseEphemeralVolume field override. |
| apis/quay/v1/quayregistry_types_test.go | Enhances override validation tests with new scenarios for ephemeral volumes. |
| apis/quay/v1/quayregistry_types.go | Adds support and validation for the ephemeral volume override. |
Comments suppressed due to low confidence (2)
pkg/middleware/middleware.go:441
- [nitpick] Consider adding an inline comment to clarify that matching the container name with 'clairAppDeploymentSuffix' is intentional to identify the Clair container for ephemeral volume injection.
if c.Name == clairAppDeploymentSuffix {
pkg/cmpstatus/clair.go:111
- The variable 'dep' is used to filter ReplicaSet owner references, but its definition isn't clearly visible in the scope of the new ephemeral volume check block. Please verify that 'dep' is properly defined and accessible here to prevent potential runtime errors.
if owner.Kind == "Deployment" && owner.Name == dep.Name {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've reviewed this pull request using the Sourcery rules engine
Add Ephemeral Volume Support for
/tmpin Managed Clair ComponentSummary
This PR introduces an optional feature for the managed Clair component, allowing users to enable a generic ephemeral volume mount at
/tmpinside the Clair container. This addresses PROJQUAY-6684 and provides a workaround for issues with/tmpstorage in certain environments.Key Features
Adds a
useEphemeralVolumeboolean override for the managed Clair component. When set totrue, a generic ephemeral volume is mounted at/tmp.Users can optionally override
storageClassNameandvolumeSize(default: 10Gi, minimum: 1Gi) for the ephemeral volume, similar to existing database components.useEphemeralVolumeis not enabled, specifyingstorageClassNameorvolumeSizetriggers a clear error in the Clair component status.PVC provisioning and error events for the ephemeral volume are surfaced in the Clair component’s status, consistent with how database PVCs are handled.
Centralizes the logic and string constants for identifying the Clair deployment, improving maintainability.
Unit test and integration test coverage has been added. Also an existing kuttl tests has been fixed.
Motivation
In environments with a heavy load and large image layers, Clair doesn't have to use the executing nodes' (often limited) local storage for unpacking individual image layers, but can rely on external storage, which can dynamically request higher capacity or faster storage. It's not a classic persistent volume because we only need this storage during the lifetime of the individual pod.
Backwards Compatibility
The new override is optional and defaults to
false, preserving existing behavior for all users not opting in.Summary by Sourcery
Add optional ephemeral volume support for Clair component to mount a PVC-backed ephemeral volume at /tmp with configurable storage class and size, including validation, status reporting, and related tests.
New Features:
useEphemeralVolumeoverride to enable an ephemeral PVC volume at/tmpin the managed Clair componentstorageClassNameandvolumeSizewith sensible defaultsEnhancements:
storageClassNameorvolumeSizeoverrides requireuseEphemeralVolume=trueand enforce a minimum 1Gi volume size/tmpvolume in the Clair component’s readiness conditionDocumentation:
useEphemeralVolumefieldTests:
/tmpvolume mount and configuration