Reviewing files that changed from the base of the PR and between dde2e90 and b8bb412.

• ANNOTATION_MIGRATION.md (1 hunks)
• README.md (1 hunks)
• contrib/test/ci/integration.yml (1 hunks)
• contrib/test/ci/vars.yml (1 hunks)
• internal/config/seccomp/notifier.go (3 hunks)
• internal/config/seccomp/seccompociartifact/seccompociartifact.go (3 hunks)
• internal/config/seccomp/seccompociartifact/seccompociartifact_test.go (4 hunks)
• internal/factory/container/container.go (8 hunks)
• internal/factory/container/container_test.go (4 hunks)
• internal/lib/checkpoint.go (1 hunks)
• internal/lib/container_server.go (7 hunks)
• internal/lib/container_server_freebsd.go (1 hunks)
• internal/lib/container_server_test.go (1 hunks)
• internal/lib/restore.go (1 hunks)
• internal/oci/container.go (2 hunks)
• internal/oci/oci_test.go (3 hunks)
• internal/oci/runtime_vm.go (1 hunks)
• pkg/annotations/annotations.go (3 hunks)
• pkg/annotations/annotations_test.go (1 hunks)
• pkg/annotations/v2/annotations.go (1 hunks)
• pkg/config/config.go (3 hunks)
• pkg/config/config_test.go (2 hunks)
• server/container_create.go (7 hunks)
• server/container_create_freebsd.go (2 hunks)
• server/container_create_linux.go (3 hunks)
• server/container_restore.go (1 hunks)
• server/container_restore_test.go (1 hunks)
• server/nri-api.go (1 hunks)
• server/sandbox_run_freebsd.go (6 hunks)
• server/sandbox_run_linux.go (9 hunks)
• server/sandbox_stop_linux.go (2 hunks)
• test/annotation_migration.bats (1 hunks)

• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: lint
• GitHub Check: docs
• GitHub Check: security-checks
• GitHub Check: build
• GitHub Check: unit / amd64 / rootless
• GitHub Check: build static / ppc64le
• GitHub Check: build static / amd64
• GitHub Check: build static / arm64
• GitHub Check: codeql-build
• GitHub Check: build static / s390x
• GitHub Check: unit / arm64 / root
• GitHub Check: binaries / arm64
• GitHub Check: unit / amd64 / root
• GitHub Check: binaries / amd64

internal/lib/checkpoint.go (1)

18-18: Importing internal annotations is consistent with checkpoint/restore usage

Switching to github.com/cri-o/cri-o/internal/annotations while continuing to use annotations.LogPath for checkpoint log archiving keeps behavior intact and aligns with the new internal-vs-public annotation split. No issues spotted here.

Also applies to: 327-351

server/sandbox_run_freebsd.go (3)

16-18: v2-based userns-mode lookup aligns with migration strategy

Using v2.GetAnnotationValue(kubeAnnotations, v2.UsernsMode) correctly prefers the new userns-mode.crio.io key while still honoring legacy values. Ignoring the found flag preserves existing behavior (empty string when unset) but now with v1/v2 compatibility.

Also applies to: 25-27, 125-129

---

293-296: Stop-signal annotation now clearly uses the public OCI image key

Annotating the infra spec with extannotations.StopSignalAnnotation (org.opencontainers.image.stopSignal) via the public pkg/annotations package cleanly separates public image metadata from internal CRI-O annotations. This matches the new internal vs external annotation split.

---

317-324: Switch to v2 PodLinux and Spoofed annotations looks correct; ensure all readers use v2 helpers*

Using v2.PodLinuxOverhead, v2.PodLinuxResources, and v2.Spoofed for newly written annotations is consistent with the *.crio.io migration and keeps this FreeBSD path aligned with the Linux implementation. Please double-check that all code paths reading these annotations have been updated to use the v2 keys or v2.GetAnnotationValue* so restored/running pods see consistent values across old and new objects.

Also applies to: 325-331, 458-461

internal/oci/runtime_vm.go (1)

44-45: Internal annotations import for Kata image source is consistent

Using internal/annotations for annotations.SomeNameOfTheImage keeps the Kata volume setup aligned with how the spec is annotated elsewhere after the migration. No issues seen with this change.

Also applies to: 118-128

server/container_restore.go (1)

17-18: Checkpoint-related annotations now sourced from internal package without behavior change

Routing CheckpointAnnotationName and annotations.Annotations through internal/annotations matches the new internal/public split while leaving the restore logic untouched. Looks good.

Also applies to: 44-45, 167-171

server/nri-api.go (1)

17-18: NRI container metadata now uses internal annotations package consistently

Switching the NRI integration to internal/annotations for ContainerID, SandboxID, and serialized Labels keeps it aligned with the rest of the annotation migration without altering behavior. No issues identified here.

Also applies to: 651-661, 684-692

server/container_restore_test.go (1)

20-20: LGTM: Import path updated to internal annotations.

The import path has been correctly updated from the public pkg/annotations to internal/annotations, aligning with the migration strategy where internal state-management annotations are separated from the public API.

pkg/config/config_test.go (1)

734-743: LGTM: Test correctly updated to v2 annotations.

The test has been properly updated to use v2.Devices instead of the legacy annotation constant, verifying that the annotation appears in allowed annotations and not in disallowed annotations.

internal/lib/container_server_freebsd.go (1)

8-8: LGTM: Import path correctly updated to internal annotations.

The import has been appropriately updated to use the internal annotations package, consistent with the migration strategy for internal state-management annotations.

internal/lib/container_server_test.go (1)

15-15: LGTM: Import path correctly migrated to internal annotations.

The import has been updated to use the internal annotations package, aligning with the broader migration strategy.

internal/config/seccomp/seccompociartifact/seccompociartifact_test.go (1)

87-88: LGTM: Tests correctly updated to use v2 annotation keys.

The tests have been properly updated to use v2.SeccompProfile with the container-specific slash separator format (/container), which aligns with the new annotation naming conventions described in the PR objectives.

Also applies to: 121-122, 134-135

server/sandbox_stop_linux.go (1)

31-31: LGTM: Correctly using v2 annotation getter with fallback support.

The code has been properly updated to use v2.GetAnnotationValue(), which provides automatic fallback to V1 annotations per the migration strategy. The ok-pattern is correctly used in the if statement.

internal/oci/container.go (1)

217-217: LGTM: Correctly using v2 Spoofed annotation key.

The annotation key has been properly updated to use v2.Spoofed, consistent with the migration to the v2 annotation API.

internal/factory/container/container_test.go (1)

388-388: Verify the dot separator usage in negative test case.

This test uses a dot separator (.) instead of the slash separator (/) used in line 363. While this appears intentional for testing that malformed annotations are not applied, it's unclear if the test objective is to:

1. Verify that annotations with the wrong separator format are ignored, OR
2. Verify that annotations for a different container name are ignored

If the intent is (2), the annotation key should use the slash separator with differentContainerName to properly test container name matching:

-	annotationKey := fmt.Sprintf("%s.%s", v2.UnifiedCgroup, differentContainerName)
+	annotationKey := fmt.Sprintf("%s/%s", v2.UnifiedCgroup, differentContainerName)

Based on learnings, please clarify the test intent to ensure the annotation format aligns with the v2 conventions.

server/container_create_linux.go (1)

32-32: v2 annotation wiring in finalizeUserMapping/specSetDevices looks correct

Importing pkg/annotations/v2 and switching to v2.GetAnnotationValue for UsernsMode and Devices preserves behavior while adding v1→v2 compatibility. Ignoring the ok flag is consistent with the previous “empty or missing means disabled” semantics.

Also applies to: 68-80, 854-870

internal/config/seccomp/notifier.go (1)

24-25: Seccomp notifier migration to v2 annotations is consistent and compatible

Using v2.GetAnnotationValue for SeccompNotifierAction in both the feature gate and NewNotifier, plus comparing against v2.SeccompNotifierActionStop, correctly adds v1/v2 compatibility without changing semantics.

Also applies to: 104-118, 161-223

server/container_create_freebsd.go (1)

22-23: FreeBSD specSetDevices correctly aligned with v2 devices annotation

Importing pkg/annotations/v2 and switching specSetDevices to use v2.GetAnnotationValue(..., v2.Devices) plus device.DevicesFromAnnotation matches the linux behavior and adds v1/v2 compatibility on FreeBSD as well.

Also applies to: 204-219

contrib/test/ci/vars.yml (1)

180-186: Kata skip list for annotation migration tests is well-integrated

The new kata_skip_annotation_migration_tests entries follow the existing pattern for skip lists, and the test titles look consistent with the annotation_migration.bats suite.

pkg/annotations/annotations_test.go (1)

9-201: Annotation tests give solid coverage of v1/v2 fallback and allowed set

The new tests thoroughly exercise GetAnnotationValue, the reverse migration map, and AllAllowedAnnotations for both v1 and v2 keys, including container-specific annotations. This should catch regressions in the migration logic early.

server/sandbox_run_linux.go (1)

1015-1027: Other v2 annotation usages in sandbox setup look good

• setupSandboxShm correctly switches to v2.GetAnnotationValue(kubeAnnotations, v2.ShmSize) while preserving the previous default and parsing behavior.
• setupSandboxResources now emits PodLinuxOverhead and PodLinuxResources under the new *.crio.io keys, matching the migration guide.
• Marking dropped infra containers with v2.Spoofed and using v2.LinkLogs for log linking are consistent with the v2 migration and will honor v1 keys via GetAnnotationValue.

These changes line up with the documented migration behavior and keep semantics intact.

Also applies to: 1194-1206, 1315-1316, 1431-1436

pkg/config/config.go (1)

49-51: Possible regression: v1 Devices annotation may be treated as disallowed by default

extannotations.AllAllowedAnnotations contains both v2 and v1 forms (including devices.crio.io and io.kubernetes.cri-o.Devices), but the default runtime handler's AllowedAnnotations only lists v2.Devices. Given how validateAllowedAndGenerateDisallowedAnnotations builds DisallowedAnnotations from AllAllowedAnnotations, the v1 key likely ends up explicitly disallowed for the default runtime.

If FilterDisallowedAnnotations matches purely on annotation key string, this would break existing pods using io.kubernetes.cri-o.Devices, contradicting the migration docs stating v1 remains supported.

Consider either:

• Allowing both versions explicitly in the default handler, e.g.:

-        AllowedAnnotations: []string{
-            extannotations.OCISeccompBPFHookAnnotation,
-            v2.Devices,
-        },
+        AllowedAnnotations: []string{
+            extannotations.OCISeccompBPFHookAnnotation,
+            v2.Devices,
+            v2.V1Devices,
+        },

or

• Adjusting the validation/filtering logic so that enabling the v2 key implicitly enables the mapped v1 key as well.

Verify the actual behavior of FilterDisallowedAnnotations with both v1 and v2 Devices annotations to confirm this doesn't break existing configs.

internal/lib/container_server.go (6)

24-36: LGTM: Import structure follows the migration pattern correctly.

The imports are well-organized:

• internal/annotations for internal state-management annotations
• extannotations alias for pkg/annotations (public API with OCI standard annotations)
• v2 alias for the new Kubernetes-recommended annotation format

---

270-281: LGTM: V2 annotation retrieval with proper fallback handling.

The usage of v2.GetAnnotationValue with v2.PodLinuxOverhead and v2.PodLinuxResources correctly implements the migration pattern. The helper automatically falls back to V1 annotations when V2 is not present.

---

293-295: LGTM: Correct handling of optional annotation.

Discarding the found boolean is appropriate here since an empty usernsMode is a valid default value when the annotation is not set.

---

358-360: LGTM: Boolean annotation check is correct.

The pattern of checking both ok (annotation exists) and spoofed == "true" is the appropriate way to handle boolean-like string annotations.

---

363-363: LGTM: OCI standard annotation correctly referenced.

StopSignalAnnotation is an OCI image-spec standard annotation (org.opencontainers.image.stopSignal), not a CRI-O annotation, so using extannotations is correct.

---

534-537: LGTM: PlatformRuntimePath annotation retrieval follows the migration pattern.

Correctly uses v2.GetAnnotationValue with the new v2 constant, falling back to empty string when not present.

internal/factory/container/container.go (4)

25-36: LGTM: Import structure correctly separates internal and external annotations.

The imports properly distinguish between:

• internal/annotations for internal container state
• extannotations for public/OCI annotations
• v2 for the new annotation format

---

205-225: LGTM: Container-specific BPF hook annotation handling is correct.

The logic properly rewrites the suffixed annotation key (e.g., io.containers.trace-syscall/container) to the base key when the container name matches, allowing per-container tracing within pods.

---

255-255: LGTM: New annotations correctly use the v2 format.

When writing annotations to new containers, using the v2 constant directly (v2.PlatformRuntimePath) is correct—new containers should use the Kubernetes-recommended format.

---

541-563: LGTM: Container-specific unified cgroup annotation handling is correct.

The key construction fmt.Sprintf("%s/%s", v2.UnifiedCgroup, containerName) follows the v2 format for container-specific annotations (unified-cgroup.crio.io/containerName), and v2.GetAnnotationValue handles the V1 fallback internally.

server/container_create.go (7)

43-43: LGTM: Import alias for v2 annotations package.

---

682-683: LGTM: Cgroup2 RW annotation check is correct.

The pattern of discarding the found boolean and comparing the value to "true" works correctly—an empty string (when not found) won't equal "true".

---

934-938: LGTM: LinkLogs annotation handling is correct.

Only proceeds with log linking when the annotation is explicitly present, which is the expected behavior.

---

984-989: LGTM: FIPS disable check combines container label and annotation.

The dual check ensures FIPS is only disabled when both the container label (FIPS_DISABLE) and the pod annotation agree.

---

1208-1210: LGTM: SELinux label skip annotation check is correct.

---

1453-1466: LGTM: User namespace ID mapping logic is correct.

This change correctly prevents adding UID/GID mappings when the container joins the sandbox's user namespace via path. As the comment explains, having both a userns path and ID mappings in the OCI spec would be conflicting—the container either joins an existing namespace or creates one with mappings, not both.

---

1484-1496: LGTM: Umask annotation handling with proper validation.

The implementation correctly:

1. Retrieves the annotation with v2 fallback
2. Validates format with a regexp (^[0-7]{1,4}$)
3. Parses as octal (base 8)

internal/config/seccomp/seccompociartifact/seccompociartifact.go (3)

7-13: LGTM: Imports updated for v2 annotations and string manipulation.

---

29-32: LGTM: Pod-level seccomp annotation constant updated to v2 format.

The constant correctly combines the v2 base annotation with the /POD suffix.

---

49-67: LGTM: Annotation lookup cascade with proper V1/V2 fallback and informative logging.

The lookup order correctly prioritizes:

1. Container-specific pod annotations
2. Pod-level annotations
3. Image annotations (with same container→pod precedence)

Using GetAnnotationValueWithKey enables logging which annotation format (V1 or V2) was actually found, which aids debugging during migration.

internal/oci/oci_test.go (3)

13-14: LGTM: Test imports correctly distinguish between annotation packages.

---

60-71: LGTM: Runtime handler allowed annotations use correct constants.

The distinction is intentional:

• v2.UsernsMode for the migrated CRI-O annotation
• extannotations.* for performance annotations and OCI/containers.io annotations that weren't part of the migration

---

158-159: LGTM: Test assertions correctly verify annotation allowlisting.

The test properly verifies that v2.Devices (not in the allowed list) is excluded while extannotations.IRQLoadBalancingAnnotation (explicitly allowed) is included.

test/annotation_migration.bats (11)

1-13: LGTM: Setup and teardown functions are properly structured.

The test file follows BATS conventions with appropriate helper loading, test directory setup, and cleanup via cleanup_test.

---

15-51: LGTM: User namespace mode annotation tests are well-structured.

Both V2 and V1 tests properly check for user namespace prerequisites and test the userns-mode annotation with the auto value.

---

53-81: LGTM: Umask annotation tests verify actual behavior.

Good practice using different umask values (022 vs 077) between V2 and V1 tests, with verification via /proc/1/status to confirm the annotation is actually applied.

---

83-115: LGTM: SHM size tests with proper verification.

The size verification logic is correct: 128Mi = 131072 KB, and using -ge accounts for potential filesystem overhead.

---

117-167: LGTM: Device annotation tests with proper precondition checks.

Good skip conditions for userNS and device availability, with verification that the mapped device is accessible inside the container.

---

169-203: LGTM: Unified cgroup annotation tests with cgroup v2 prerequisite.

Tests properly skip on non-cgroup-v2 systems and verify the annotation is accepted by running a pod successfully.

---

205-257: LGTM: Seccomp profile container-specific annotation tests.

Good demonstration of the /POD suffix pattern for both V2 and V1 formats, with a minimal but valid seccomp profile.

---

259-299: LGTM: Cgroup2 RW mount annotation tests with verification.

Tests properly verify the annotation effect by checking /proc/mounts for the rw flag on cgroup2.

---

301-335: LGTM: SELinux label skip annotation tests with proper environment check.

Proper SELinux availability detection using getenforce, and tests verify the annotation is accepted.

---

337-361: LGTM: Container-specific annotation test with slash separator.

Good test coverage for the standardized slash-separated container-specific annotation format (unified-cgroup.crio.io/containerName).

---

363-557: LGTM: Comprehensive test coverage for remaining annotations.

Tests for seccomp-notifier-action, disable-fips, link-logs, pod-linux-overhead, pod-linux-resources, and platform-runtime-path follow consistent patterns with appropriate skip conditions. All 15 migrated annotations are now covered.

pkg/annotations/annotations.go (4)

5-6: LGTM: V2 package import added.

The import follows the convention of using v2 as the alias for clarity.

---

9-19: LGTM: Backward-compatible wrapper with clear deprecation guidance.

The wrapper delegates to v2.GetAnnotationValue while providing clear deprecation notice and documentation for the migration path.

---

21-125: LGTM: Constants properly re-exported with deprecation notices.

All migrated annotations are aliased to v2.V1* constants with consistent deprecation documentation recommending both the V1 constant in v2 and the preferred V2 format.

---

127-161: LGTM: AllAllowedAnnotations properly composed for migration period.

The nested append structure correctly includes base annotations, V2 recommended annotations, and V1 deprecated annotations to support both formats during the transition.

pkg/annotations/v2/annotations.go (6)

1-57: LGTM: V2 annotation constants follow Kubernetes-recommended naming.

All V2 annotations use the *.crio.io format as per Kubernetes conventions, with clear documentation for each annotation's purpose.

---

58-138: LGTM: V1 deprecated constants preserve legacy format.

V1 constants correctly preserve the legacy io.kubernetes.cri-o.* format (and the special seccomp-profile.kubernetes.cri-o.io format) with proper deprecation notices.

---

140-158: LGTM: Complete reverse migration map for V2→V1 lookups.

All 15 migrated annotations are correctly mapped for backward compatibility lookups.

---

160-201: LGTM: Annotation lookup with V2-first precedence and V1 fallback.

The lookup functions correctly prioritize V2 annotations, fall back to V1 using the migration map, and handle container-specific annotations. The GetAnnotationValueWithKey variant is useful for logging which key was actually found.

---

203-223: LGTM: Container-specific V1 key derivation handles both separators.

The function correctly extracts the suffix (including separator) from V2 keys and appends it to the V1 base. Supporting both slash and dot separators ensures compatibility during the transition.

---

225-261: LGTM: Complete annotation lists for V2 and V1.

Both slices contain all 15 migrated annotations, alphabetically sorted for maintainability.

Reviewing files that changed from the base of the PR and between b8bb412 and 82dc69d.

• ANNOTATION_MIGRATION.md (1 hunks)
• README.md (1 hunks)
• contrib/test/ci/integration.yml (1 hunks)
• contrib/test/ci/vars.yml (1 hunks)
• internal/config/seccomp/notifier.go (3 hunks)
• internal/config/seccomp/seccompociartifact/seccompociartifact.go (3 hunks)
• internal/config/seccomp/seccompociartifact/seccompociartifact_test.go (4 hunks)
• internal/factory/container/container.go (8 hunks)
• internal/factory/container/container_test.go (4 hunks)
• internal/lib/checkpoint.go (1 hunks)
• internal/lib/container_server.go (7 hunks)
• internal/lib/container_server_freebsd.go (1 hunks)
• internal/lib/container_server_test.go (1 hunks)
• internal/lib/restore.go (1 hunks)
• internal/oci/container.go (2 hunks)
• internal/oci/oci_test.go (3 hunks)
• internal/oci/runtime_vm.go (1 hunks)
• internal/runtimehandlerhooks/high_performance_hooks_linux.go (4 hunks)
• internal/runtimehandlerhooks/high_performance_hooks_test.go (9 hunks)
• internal/runtimehandlerhooks/runtime_handler_hooks_linux.go (3 hunks)
• pkg/annotations/annotations.go (1 hunks)
• pkg/annotations/annotations_test.go (1 hunks)
• pkg/annotations/v2/annotations.go (1 hunks)
• pkg/config/config.go (3 hunks)
• pkg/config/config_test.go (2 hunks)
• server/container_create.go (7 hunks)
• server/container_create_freebsd.go (2 hunks)
• server/container_create_linux.go (3 hunks)
• server/container_restore.go (1 hunks)
• server/container_restore_test.go (1 hunks)
• server/nri-api.go (1 hunks)
• server/sandbox_run_freebsd.go (6 hunks)
• server/sandbox_run_linux.go (9 hunks)
• server/sandbox_stop_linux.go (2 hunks)
• test/annotation_migration.bats (1 hunks)

• server/container_create_freebsd.go
• internal/config/seccomp/notifier.go
• contrib/test/ci/vars.yml
• server/container_create_linux.go
• pkg/config/config.go
• README.md
• server/container_create.go
• server/container_restore_test.go
• internal/lib/container_server_freebsd.go
• internal/lib/container_server_test.go
• internal/lib/restore.go
• pkg/config/config_test.go
• internal/lib/checkpoint.go
• server/sandbox_run_linux.go

• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: integration / conmon-rs / crun / amd64
• GitHub Check: integration / conmon / crun / amd64
• GitHub Check: critest / conmon / crun / arm64
• GitHub Check: integration / conmon / crun / arm64
• GitHub Check: integration / userns / crun / amd64
• GitHub Check: critest / conmon-rs / crun / arm64
• GitHub Check: critest / conmon-rs / crun / amd64
• GitHub Check: critest / conmon / crun / amd64
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: security-checks
• GitHub Check: build
• GitHub Check: codeql-build
• GitHub Check: unit / amd64 / rootless
• GitHub Check: build static / arm64
• GitHub Check: build static / amd64
• GitHub Check: unit / amd64 / root
• GitHub Check: unit / arm64 / root
• GitHub Check: build static / s390x
• GitHub Check: build static / ppc64le
• GitHub Check: docs
• GitHub Check: lint

server/container_restore.go (1)

17-22: Import switch to internal annotations is consistent with the new split

Using github.com/cri-o/cri-o/internal/annotations here for CheckpointAnnotationName and Annotations matches the intent to keep state-management keys internal; behavior remains unchanged.

pkg/annotations/annotations_test.go (1)

9-170: V1/V2 fallback and migration-map test coverage looks solid

The table-driven TestGetAnnotationValue plus TestReverseAnnotationMigrationMap exercise:

• V2-only, V1-only, and both-present cases with correct V2 precedence,
• LinkLogs and DisableFIPS,
• Container-specific annotations in both dot- and slash-separated forms, including V1 fallbacks.

This gives good confidence that v2.GetAnnotationValue and the reverse mapping behave as intended.

internal/oci/oci_test.go (1)

48-71: AllowedAnnotations migration in tests matches v2 constants and expected behavior

Runtime handler configs and the AllowedAnnotations test now consistently use v2 keys (UsernsMode, CPULoadBalancing, IRQLoadBalancing, CPUQuota, OCISeccompBPFHook), and the assertion that foundAnn excludes v2.Devices while including v2.IRQLoadBalancing preserves the prior intent of the test. No issues here.

Also applies to: 147-159

internal/oci/container.go (1)

28-33: Spoofed-container annotation migrated cleanly to v2 key

NewSpoofedContainer now marks spoofed containers via v2.Spoofed in the annotations map, which aligns with the v2 annotation namespace without changing any other behavior. This looks correct.

Also applies to: 205-228

internal/oci/runtime_vm.go (1)

44-48: Using internal annotations package for SomeNameOfTheImage is appropriate

addVolumeMountsToCreateRequest now reads annotations.SomeNameOfTheImage from internal/annotations, keeping this state-management key internal while preserving the existing behavior when constructing KataVirtualVolume. No further changes needed.

Also applies to: 118-128

contrib/test/ci/integration.yml (1)

48-66: Kata skip wiring for annotation_migration.bats matches existing pattern

Adding

+ ['annotation_migration.bats'] | product(kata_skip_annotation_migration_tests) \

into the kata skip loop is consistent with how other .bats suites are handled, and the file name is kept relative (just annotation_migration.bats), which aligns with the existing integration-test path conventions.

Based on learnings, this follows the “use relative test paths” guidance.

server/nri-api.go (1)

17-17: LGTM!

The import path change from pkg/annotations to internal/annotations correctly separates internal state-management annotations (like ContainerID, SandboxID, Labels) from the public API annotations. This aligns with the PR's objective to reorganize annotations by scope.

server/sandbox_stop_linux.go (1)

17-17: LGTM!

The migration to v2.GetAnnotationValue(sbAnnotations, v2.LinkLogs) correctly implements the new annotation lookup pattern. This helper function automatically checks for the V2 annotation (link-logs.crio.io) first, then falls back to the V1 annotation if not found, ensuring backward compatibility during the migration phase.

Also applies to: 31-31

ANNOTATION_MIGRATION.md (2)

36-52: Verify the migration table is complete.

The table lists 15 annotations with V1 equivalents. This matches the PR objective ("all 15 migrated annotations"). The mapping looks correct and the table is well-formatted.

---

1-228: Documentation is comprehensive and well-structured.

The migration guide effectively covers:

• Clear format comparison between old and new conventions
• Complete migration table with status
• Backward compatibility guarantees with precedence rules
• Developer-focused code examples using v2.GetAnnotationValue()
• User-facing YAML examples for common annotations
• Testing instructions

internal/config/seccomp/seccompociartifact/seccompociartifact_test.go (1)

15-15: LGTM!

The test file correctly migrates to use v2.SeccompProfile for container-specific annotation lookups. The pattern v2.SeccompProfile + "/container" aligns with the PR's standardization of container-specific annotations using a slash separator (e.g., seccomp-profile.crio.io/container).

Also applies to: 87-87, 121-121, 134-134

server/sandbox_run_freebsd.go (4)

16-16: LGTM!

The import organization correctly separates concerns:

• internal/annotations for internal state-management annotations
• extannotations (pkg/annotations) for backward compatibility with legacy annotation constants
• v2 (pkg/annotations/v2) for new annotation format and helpers

Also applies to: 25-26

---

127-128: LGTM!

Correctly uses v2.GetAnnotationValue to read the user namespace mode with automatic V1 fallback, ensuring backward compatibility during migration.

---

323-323: LGTM!

When writing annotations to the spec, using V2 constants directly (v2.PodLinuxOverhead, v2.PodLinuxResources, v2.Spoofed) is the correct approach. This ensures new annotations are written in the Kubernetes-recommended format while reading operations (like line 127) use the fallback helper for backward compatibility.

Also applies to: 330-330, 460-460

---

294-296: LGTM!

The usage of extannotations.StopSignalAnnotation is correct for the OCI image-spec stop signal annotation (org.opencontainers.image.stopSignal). This is an external standard annotation that doesn't follow the CRI-O migration pattern.

internal/runtimehandlerhooks/runtime_handler_hooks_linux.go (3)

9-9: LGTM!

Import path correctly updated to the v2 annotations package.

---

90-106: The V1 annotation naming concern appears to be based on incorrect assumptions.

CRI-O documentation indicates the annotation is named cpu-load-balancing.crio.io (a single, consistent name), not separate V1 and V2 variants like io.kubernetes.cri-o.CPULoadBalancing. Without access to verify what crioann.CPULoadBalancing actually represents in the code, the review comment's premise about V1 vs. V2 annotation naming incompatibility cannot be confirmed. If crioann.CPULoadBalancing maps to the documented annotation name, there is no backward compatibility issue as described.

---

75-88: Consider adding documentation or verification of V1 annotation support if legacy formats are in use.

The function checks for crioann.* constant prefixes. If your deployment supports legacy annotation formats from earlier CRI-O versions for these performance settings, ensure they are also detected. However, verify first whether actual V1 format variants exist and are actively used in your environment, as CRI-O's modern annotation structure is consistent across versions.

internal/lib/container_server.go (3)

24-36: LGTM!

Both the internal annotations and v2 annotations packages are correctly imported for the annotation migration.

---

269-279: LGTM!

Using v2.GetAnnotationValue() correctly provides backward compatibility with V1 annotations while preferring V2.

---

292-294: LGTM!

UsernsMode correctly uses v2.GetAnnotationValue() for V1 fallback support.

internal/runtimehandlerhooks/high_performance_hooks_test.go (6)

25-25: LGTM!

Import path correctly updated to use v2 annotations package.

---

426-432: LGTM!

Test correctly uses v2 IRQLoadBalancing constant for annotation key.

---

1258-1264: LGTM!

Test correctly uses v2 CPUShared constant with container-specific suffix format.

---

1489-1504: LGTM!

Test correctly uses v2 IRQLoadBalancing annotation constant.

---

1566-1583: LGTM!

Test correctly uses v2 IRQLoadBalancing constant in AllowedAnnotations configuration.

---

1669-1693: LGTM!

Test correctly uses both IRQLoadBalancing and CPULoadBalancing v2 constants to verify runtime configuration behavior.

test/annotation_migration.bats (12)

1-13: LGTM!

Standard BATS test structure with proper setup and teardown functions using setup_test and cleanup_test helpers. File correctly uses .bats extension as per coding guidelines.

---

15-51: LGTM!

Good test coverage for userns-mode annotation with both V2 and V1 formats. Proper skip conditions for missing user namespace configuration.

---

53-81: LGTM!

Good test coverage for umask annotation verifying the umask is correctly applied to the container process.

---

83-115: LGTM!

Good test coverage for shm-size annotation with actual verification of shared memory size.

---

117-167: LGTM!

Good test coverage for devices annotation with proper skip conditions and device accessibility verification.

---

169-203: LGTM!

Good test coverage for unified-cgroup annotation with proper cgroup v2 availability check.

---

205-257: LGTM!

Good test coverage for container-specific seccomp-profile annotation using the slash separator format (e.g., seccomp-profile.crio.io/POD).

---

259-299: LGTM!

Good test coverage for cgroup2-mount-hierarchy-rw annotation with actual mount verification.

---

301-335: LGTM!

Good test coverage for try-skip-volume-selinux-label annotation with proper SELinux availability check.

---

337-361: LGTM!

Good test for container-specific unified-cgroup annotation with slash separator format.

---

399-557: LGTM!

Good test coverage for disable-fips, link-logs, pod-linux-overhead, pod-linux-resources, and platform-runtime-path annotations with both V1 and V2 formats.

---

363-397: Verify checkseccomp helper function exists.

The test uses checkseccomp but this helper function definition should be confirmed to exist in test/helpers.bash or test/common.bash.

internal/runtimehandlerhooks/high_performance_hooks_linux.go (5)

29-29: LGTM!

Import path correctly updated to use v2 annotations package.

---

401-411: Same V1 fallback concern for C-States and FreqGovernor.

shouldCStatesBeConfigured and shouldFreqGovernorBeConfigured also use direct v2 constant lookups without V1 fallback.

---

417-422: LGTM with same caveat.

requestedSharedCPUs constructs the container-specific key using v2 constant. The slash separator format is correct for v2 annotations.

---

1519-1522: LGTM!

isRequestedHousekeepingCPUs correctly uses v2 IRQLoadBalancing constant.

---

373-399: Verify V1 annotation handling in the runtime path.

These functions (shouldCPULoadBalancingBeDisabled, shouldCPUQuotaBeDisabled, shouldIRQLoadBalancingBeDisabled) directly check annotations[crioannotations.CPULoadBalancing] etc., without apparent V1 fallback. If users specify V1 annotations in their pod spec (e.g., io.kubernetes.cri-o.CPULoadBalancing), these checks may fail to detect them.

Confirm whether:

1. Annotations are normalized to V2 format upstream before reaching these functions, or
2. These functions should use a helper like v2.GetAnnotationValue() for V1 fallback support.

pkg/annotations/annotations.go (2)

3-7: Alias import for v2 annotations looks good

Using an alias import for pkg/annotations/v2 keeps the transition clear and avoids churn on existing annotations imports. No issues here.

---

21-133: Re-exported v1/v2 annotation constants maintain compatibility

The pattern of re‑exporting v1 keys (where they exist) and pointing deprecation comments at the corresponding v2 constants is consistent and keeps the public surface stable for existing users. I don’t see mismatches between the names and the referenced v2 symbols.

internal/config/seccomp/seccompociartifact/seccompociartifact.go (2)

29-35: Pod-level seccomp annotation constant correctly aligned to v2

Using SeccompProfilePodAnnotation = v2.SeccompProfile + "/POD" keeps the POD suffix logic centralized and naturally shares the v1 fallback behavior via v2 helpers. This is a clean migration.

---

49-68: Seccomp annotation lookup flow and logging with v2 helpers look sound

The new flow:

• Builds containerKey as seccomp-profile.crio.io/<containerName>.
• Uses GetAnnotationValueWithKey across pod and image annotations, so v2 is preferred while v1 keys still work.
• Derives baseKey via TrimSuffix for /POD and "/"+containerName to log the logical base key regardless of format.

Lookup precedence and behavior are preserved, and the logging accurately reflects whichever key was actually used. I don’t see edge cases that would break under normal Kubernetes container names.

pkg/annotations/v2/annotations.go (2)

3-169: V2 and V1 annotation constants are coherent and match the migration intent

The v2 constants consistently use the *.crio.io form, and the V1 constants retain the legacy io.kubernetes.cri-o.* / seccomp-profile.kubernetes.cri-o.io spellings with clear deprecation notes. The separate SeccompNotifierActionStop value being non-annotation is also correctly scoped here. I don’t see inconsistencies between the comments and the literal strings.

---

256-300: AllAnnotations / AllV1Annotations sets look appropriate for allowed-annotations aggregation

The v2 list covers all of the v2 keys that CRI-O itself owns (excluding non-annotation values like SeccompNotifierActionStop), and AllV1Annotations mirrors the subset of v1 keys that have v2 counterparts and are still supported. This lines up with how pkg/annotations.AllAllowedAnnotations composes the final allowed set, and should make future additions less error-prone.

Reviewing files that changed from the base of the PR and between 82dc69d and 66091fb.

• ANNOTATION_MIGRATION.md (1 hunks)
• README.md (1 hunks)
• contrib/test/ci/integration.yml (1 hunks)
• contrib/test/ci/vars.yml (1 hunks)
• internal/config/seccomp/notifier.go (3 hunks)
• internal/config/seccomp/seccompociartifact/seccompociartifact.go (3 hunks)
• internal/config/seccomp/seccompociartifact/seccompociartifact_test.go (4 hunks)
• internal/factory/container/container.go (8 hunks)
• internal/factory/container/container_test.go (4 hunks)
• internal/lib/checkpoint.go (1 hunks)
• internal/lib/container_server.go (7 hunks)
• internal/lib/container_server_freebsd.go (1 hunks)
• internal/lib/container_server_test.go (1 hunks)
• internal/lib/restore.go (1 hunks)
• internal/oci/container.go (2 hunks)
• internal/oci/oci_test.go (3 hunks)
• internal/oci/runtime_vm.go (1 hunks)
• internal/runtimehandlerhooks/high_performance_hooks_linux.go (4 hunks)
• internal/runtimehandlerhooks/high_performance_hooks_test.go (9 hunks)
• internal/runtimehandlerhooks/runtime_handler_hooks_linux.go (3 hunks)
• pkg/annotations/annotations.go (1 hunks)
• pkg/annotations/annotations_test.go (1 hunks)
• pkg/annotations/v2/annotations.go (1 hunks)
• pkg/config/config.go (3 hunks)
• pkg/config/config_test.go (2 hunks)
• server/container_create.go (7 hunks)
• server/container_create_freebsd.go (2 hunks)
• server/container_create_linux.go (3 hunks)
• server/container_restore.go (1 hunks)
• server/container_restore_test.go (1 hunks)
• server/nri-api.go (1 hunks)
• server/sandbox_run_freebsd.go (6 hunks)
• server/sandbox_run_linux.go (9 hunks)
• server/sandbox_stop_linux.go (2 hunks)
• test/annotation_migration.bats (1 hunks)

• server/container_restore.go
• internal/oci/runtime_vm.go

• internal/runtimehandlerhooks/high_performance_hooks_test.go
• server/container_restore_test.go
• ANNOTATION_MIGRATION.md
• server/container_create_linux.go
• internal/config/seccomp/notifier.go
• internal/lib/container_server_test.go
• server/container_create_freebsd.go
• test/annotation_migration.bats
• internal/oci/container.go
• server/container_create.go
• internal/lib/restore.go
• server/sandbox_stop_linux.go
• internal/config/seccomp/seccompociartifact/seccompociartifact_test.go
• README.md
• server/nri-api.go
• internal/config/seccomp/seccompociartifact/seccompociartifact.go
• server/sandbox_run_freebsd.go

• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: binaries / arm64
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: docs
• GitHub Check: security-checks
• GitHub Check: build static / ppc64le
• GitHub Check: codeql-build
• GitHub Check: unit / arm64 / root
• GitHub Check: lint
• GitHub Check: unit / amd64 / rootless
• GitHub Check: build static / arm64
• GitHub Check: build
• GitHub Check: build static / amd64
• GitHub Check: build static / s390x
• GitHub Check: unit / amd64 / root

contrib/test/ci/vars.yml (1)

180-186: Verify test name strings are exact matches in annotation_migration.bats.

The variable definition follows the established pattern and covers v1/v2 scenarios across four annotation types (cgroup2-mount-hierarchy-rw, userns-mode, umask, unified-cgroup). The last test specifically references container slash-separator syntax, which aligns with the PR's standardization of container-specific annotations.

However, the skip logic in integration.yml line 52 uses regex matching on item.1, so test names must be exact matches (including quote style and punctuation).

Use the verification script from the integration.yml review comment to confirm all six test names match their counterparts in test/annotation_migration.bats.

contrib/test/ci/integration.yml (1)

65-65: Verify that test names in kata_skip_annotation_migration_tests configuration exactly match actual test case names in annotation_migration.bats.

Line 65 correctly extends the kata skip loop using the same pattern as other test suites (cgroups, command, etc.). The skip mechanism requires exact string matches between configured test names and actual test case names defined in the .bats file; mismatches will silently fail to apply skips. Confirm that all test names listed in kata_skip_annotation_migration_tests (in the configuration file) correspond exactly to the test "..." declarations in test/annotation_migration.bats.

internal/lib/container_server_freebsd.go (2)

8-8: LGTM: Import aligns with internal annotation migration.

The import change from pkg/annotations to internal/annotations correctly reflects the PR's migration of internal state annotations to the internal/annotations package.

---

16-17: Verify that direct map access is appropriate for these annotations.

The code uses direct annotation map access (spec.Annotations[annotations.HostNetwork] and spec.Annotations[annotations.SandboxID]) rather than GetAnnotationValue(). According to the PR summary, the migration strategy uses GetAnnotationValue() to provide V2 precedence with V1 fallback for migrated annotations.

Please confirm whether HostNetwork and SandboxID are:

1. Internal-only annotations that don't require V2/V1 compatibility handling (in which case direct access is correct), or
2. Migrated annotations that should use GetAnnotationValue() for proper V2 precedence and V1 fallback

internal/oci/oci_test.go (1)

12-15: Runtime AllowedAnnotations migration to v2 constants looks correct

Switching AllowedAnnotations and the corresponding expectations to the v2 constants (v2.UsernsMode, v2.CPULoadBalancing, v2.IRQLoadBalancing, v2.CPUQuota, v2.OCISeccompBPFHook) is consistent with the new annotations API. The assertions that foundAnn does not contain v2.Devices but does contain v2.IRQLoadBalancing still exercise the intended behavior without changing semantics.

Also applies to: 48-60, 61-71, 147-159

internal/lib/checkpoint.go (1)

18-21: Importing internal annotations for LogPath is a pure wiring change

Using github.com/cri-o/cri-o/internal/annotations here to access annotations.LogPath keeps the checkpoint/export logic unchanged while aligning with the new internal vs. v2 split of annotations. No behavioral or compatibility issues in this file.

Also applies to: 321-351

pkg/config/config_test.go (1)

14-18: Config runtime tests correctly track v2.Devices annotation

Updating:

• AllowedAnnotations: []string{v2.Devices} and
• The expectations to require v2.Devices in AllowedAnnotations but not in DisallowedAnnotations

keeps this test aligned with the new v2 annotation set and the validateAllowedAndGenerateDisallowedAnnotations logic. The test still exercises the intended contract without changing semantics.

Also applies to: 731-744

internal/runtimehandlerhooks/runtime_handler_hooks_linux.go (1)

3-11: High‑performance hooks now consistently use v2 annotation constants

Using the v2 constants (crioann.CPULoadBalancing, crioann.CPUQuota, crioann.IRQLoadBalancing, crioann.CPUCStates, crioann.CPUFreqGovernor, crioann.CPUShared) for:

• prefix detection in highPerformanceAnnotationsSpecified, and
• equality checks in cpuLoadBalancingAllowed,

keeps the runtime hooks aligned with the new annotation namespace while preserving behavior (the underlying key strings stay the same). No issues spotted here.

Also applies to: 75-88, 90-109

internal/factory/container/container.go (2)

25-36: Spec annotations correctly migrated to v2 keys and internal constants

In SpecAddAnnotations:

• The OCI seccomp BPF hook rewrite now uses v2.OCISeccompBPFHook and safely initializes c.config.Annotations if it’s nil before writing to it, avoiding a possible panic on a nil annotations map.
• The runtime path and stop‑signal annotations are emitted under the new v2 keys (v2.PlatformRuntimePath and v2.StopSignal), matching the migration strategy while leaving other internal annotations under internal/annotations.

This all looks coherent with the new annotation model and improves robustness around nil maps.

Also applies to: 199-229, 258-259, 311-314

---

533-547: Unified cgroup resource parsing uses v2 key + iterator; Go version support requirement is sound

AddUnifiedResourcesFromAnnotations now:

• Builds the lookup key as fmt.Sprintf("%s/%s", v2.UnifiedCgroup, containerName) and retrieves the value via v2.GetAnnotationValue, honoring both the new v2 key and any mapped v1 key for the same container.
• Iterates over entries with for entry := range strings.SplitSeq(annotation, ";"), which avoids slice allocations and skips empty segments before parsing key=value pairs. strings.SplitSeq returns an iter.Seq[string] iterator available in Go 1.24+ and provides the same behavior as strings.Split without constructing the full slice.
• Preserves existing behavior for base64‑encoded vs. raw values and keeps the error message tied to the unified‑cgroup annotation key.

Ensure that go.mod targets Go 1.24 or later for this code to compile.

internal/lib/container_server.go (4)

269-280: Consistent v2 annotation migration for pod resources.

The migration to v2.GetAnnotationValue for PodLinuxOverhead and PodLinuxResources correctly provides V1 fallback while preferring V2 annotations. Error messages appropriately reference the v2 keys.

---

292-294: UsernsMode retrieval migrated to v2.

The change correctly uses v2.GetAnnotationValue for UsernsMode, ensuring backward compatibility with V1 annotations.

---

357-364: Spoofed and StopSignal now use v2.GetAnnotationValue.

Both annotations now use the v2 getter, which provides proper V1 fallback. This addresses the previous review concern about StopSignal using direct map access.

---

535-555: LoadContainer annotation retrieval migrated to v2.

Both PlatformRuntimePath and StopSignal now use v2.GetAnnotationValue, ensuring containers saved with V1 annotations will be loaded correctly. The pattern is consistent with LoadSandbox.

server/sandbox_run_linux.go (6)

496-501: UsernsMode retrieval and deprecation warning.

The migration to v2.GetAnnotationValue is correct. The deprecation warning at line 498 references the old annotation name io.kubernetes.cri-o.userns-mode, which is appropriate since users providing that annotation should be warned to migrate.

---

1014-1021: ShmSize annotation retrieval migrated to v2.

Correctly uses v2.GetAnnotationValue for reading the shm size annotation with V1 fallback.

---

1073-1075: StopSignal written using v2 constant.

New sandboxes will have the StopSignal annotation written using the v2 key (org.opencontainers.image.stopSignal), which is the canonical OCI format.

---

1193-1205: Pod resources annotations written using v2 keys.

PodLinuxOverhead and PodLinuxResources are correctly written using v2 annotation keys for new sandboxes.

---

1314-1314: Spoofed annotation written using v2 key.

New dropped infra containers will have the Spoofed annotation in v2 format.

---

1429-1434: LinkLogs annotation retrieval migrated to v2.

Correctly uses v2.GetAnnotationValue for the LinkLogs feature with V1 fallback.

internal/runtimehandlerhooks/high_performance_hooks_linux.go (5)

29-29: Import alias updated to v2 annotations package.

The crioannotations alias now correctly points to the v2 package.

---

373-399: High-performance hook annotation checks updated to v2 keys.

The annotation key references (CPULoadBalancing, CPUQuota, IRQLoadBalancing) are correctly updated to use v2 constants. These annotations don't have V1 equivalents requiring migration (they were already in the v2-like format), so direct map access is appropriate here.

---

401-411: C-states and frequency governor annotation checks updated.

shouldCStatesBeConfigured and shouldFreqGovernorBeConfigured correctly use the v2 annotation constants.

---

417-422: Shared CPUs annotation key updated to v2.

The container-specific annotation key construction (crioannotations.CPUShared + "/" + cName) correctly uses the v2 base key with the slash separator convention.

---

1520-1522: Housekeeping CPUs check updated to v2 key.

isRequestedHousekeepingCPUs correctly references the v2 IRQLoadBalancing constant.

pkg/annotations/annotations.go (3)

9-19: GetAnnotationValue wrapper for backwards compatibility.

The wrapper correctly delegates to v2.GetAnnotationValue and is appropriately marked as deprecated. This maintains backwards compatibility for existing callers while guiding them to migrate.

---

21-133: V1 annotation constants aliased to v2 equivalents.

All legacy annotation constants are now aliased to their v2 package equivalents with clear deprecation notices. This approach:

• Maintains backwards compatibility for code using the old constants
• Provides clear migration guidance in the deprecation comments
• Centralizes the actual values in the v2 package

---

135-160: AllAllowedAnnotations composition is well-structured.

The composition correctly includes:

1. External annotations (rdt, runc, crun)
2. V2 annotations (recommended)
3. V1 annotations (deprecated, for backwards compatibility)

This ensures all annotation forms are allowed during the migration period.

pkg/annotations/v2/annotations.go (4)

1-88: Well-structured V2 annotation constants.

The V2 annotations follow the Kubernetes-recommended *.crio.io naming convention. Notable exceptions that use external/standard formats:

• StopSignal: Uses OCI standard org.opencontainers.image.stopSignal
• OCISeccompBPFHook: Uses existing external format io.containers.trace-syscall

This is appropriate as these are cross-project standards.

---

171-189: Reverse migration map for efficient V1 fallback.

The reverseAnnotationMigrationMap correctly maps V2 keys to their V1 equivalents. Note that annotations like CPULoadBalancing, CPUQuota, IRQLoadBalancing, etc. are intentionally omitted as they don't have legacy V1 equivalents requiring migration.

---

209-232: GetAnnotationValueWithKey correctly implements V1 fallback.

The function:

1. Prefers V2 annotation
2. Falls back to V1 via the reverse map
3. Handles container-specific annotations

The logic is sound and efficient.

---

256-300: Annotation lists are comprehensive and consistent.

AllAnnotations includes all V2 annotations, and AllV1Annotations includes all deprecated V1 annotations. These lists are used by the wrapper package to construct AllAllowedAnnotations.

Reviewing files that changed from the base of the PR and between 66091fb and b4b77bf.

• ANNOTATION_MIGRATION.md (1 hunks)
• README.md (1 hunks)
• contrib/test/ci/integration.yml (1 hunks)
• contrib/test/ci/vars.yml (1 hunks)
• internal/config/seccomp/notifier.go (3 hunks)
• internal/config/seccomp/seccompociartifact/seccompociartifact.go (3 hunks)
• internal/config/seccomp/seccompociartifact/seccompociartifact_test.go (4 hunks)
• internal/factory/container/container.go (8 hunks)
• internal/factory/container/container_test.go (4 hunks)
• internal/lib/checkpoint.go (1 hunks)
• internal/lib/container_server.go (7 hunks)
• internal/lib/container_server_freebsd.go (1 hunks)
• internal/lib/container_server_test.go (1 hunks)
• internal/lib/restore.go (1 hunks)
• internal/oci/container.go (2 hunks)
• internal/oci/oci_test.go (3 hunks)
• internal/oci/runtime_vm.go (1 hunks)
• internal/runtimehandlerhooks/high_performance_hooks_linux.go (4 hunks)
• internal/runtimehandlerhooks/high_performance_hooks_test.go (9 hunks)
• internal/runtimehandlerhooks/runtime_handler_hooks_linux.go (3 hunks)
• pkg/annotations/annotations.go (1 hunks)
• pkg/annotations/annotations_test.go (1 hunks)
• pkg/annotations/v2/annotations.go (1 hunks)
• pkg/config/config.go (4 hunks)
• pkg/config/config_test.go (2 hunks)
• server/container_create.go (7 hunks)
• server/container_create_freebsd.go (2 hunks)
• server/container_create_linux.go (3 hunks)
• server/container_restore.go (1 hunks)
• server/container_restore_test.go (1 hunks)
• server/nri-api.go (1 hunks)
• server/sandbox_run_freebsd.go (6 hunks)
• server/sandbox_run_linux.go (9 hunks)
• server/sandbox_stop_linux.go (2 hunks)
• test/annotation_migration.bats (1 hunks)

• server/container_restore_test.go
• pkg/config/config.go
• server/nri-api.go
• server/container_create_linux.go
• internal/lib/container_server_test.go
• server/sandbox_stop_linux.go
• server/sandbox_run_freebsd.go
• server/container_restore.go
• internal/lib/restore.go
• internal/lib/checkpoint.go
• internal/lib/container_server_freebsd.go
• internal/oci/oci_test.go
• internal/oci/runtime_vm.go
• internal/factory/container/container_test.go
• test/annotation_migration.bats
• internal/config/seccomp/seccompociartifact/seccompociartifact_test.go

• GitHub Check: integration / conmon / crun / amd64
• GitHub Check: integration / userns / crun / amd64
• GitHub Check: integration / conmon-rs / crun / amd64
• GitHub Check: integration / conmon / crun / arm64
• GitHub Check: critest / conmon-rs / crun / amd64
• GitHub Check: critest / conmon-rs / crun / arm64
• GitHub Check: critest / conmon / crun / arm64
• GitHub Check: critest / conmon / crun / amd64
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
• GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
• GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
• GitHub Check: build static / ppc64le
• GitHub Check: build static / amd64
• GitHub Check: build static / s390x
• GitHub Check: build static / arm64
• GitHub Check: codeql-build
• GitHub Check: security-checks
• GitHub Check: unit / arm64 / root
• GitHub Check: unit / amd64 / rootless
• GitHub Check: unit / amd64 / root
• GitHub Check: docs
• GitHub Check: build
• GitHub Check: lint

pkg/config/config_test.go (2)

14-18: Importing v2 annotations looks correct

Switching to the v2 annotations package with an alias and using it only where needed in tests is consistent with the new canonical namespace and keeps imports clear.

---

730-744: Test now correctly targets the v2 Devices annotation

Using v2.Devices in AllowedAnnotations and asserting it remains allowed and not disallowed matches the new annotation naming and preserves the intended behavior of ValidateRuntimes().

internal/oci/container.go (1)

32-32: LGTM!

The import alias change to v2 and the use of v2.Spoofed for the spoofed container annotation are consistent with the V2 annotation migration pattern across the codebase.

Also applies to: 217-217

pkg/annotations/annotations_test.go (1)

9-130: Well-structured test coverage for the annotation migration.

The table-driven tests comprehensively cover:

• V2 annotation lookup
• V1 fallback behavior
• V2 precedence when both formats are present
• Container-specific annotations with both dot and slash separators

The //nolint:staticcheck directives are appropriate for testing deprecated V1 annotations.

internal/config/seccomp/notifier.go (1)

24-24: LGTM!

The migration to V2 annotations is correctly implemented:

• v2.GetAnnotationValue provides automatic V1 fallback for backward compatibility
• Error messages reference the new V2 key (v2.SeccompNotifierAction)
• The SeccompNotifierActionStop constant is properly referenced from the v2 package

Also applies to: 116-118, 211-222

internal/factory/container/container.go (3)

223-228: Good defensive nil check added.

The nil map check before setting annotation values prevents potential panics when c.config.Annotations is uninitialized.

---

204-232: LGTM!

The migration to V2 annotation constants is correctly applied:

• v2.OCISeccompBPFHook for the OCI seccomp BPF hook annotation
• v2.PlatformRuntimePath for platform-specific runtime paths
• v2.StopSignal for the OCI stop signal annotation
• The container-specific key format using "/" separator aligns with the standardized V2 convention

Also applies to: 258-258, 313-313

---

544-547: Verify strings.SplitSeq usage in the specified lines.

strings.SplitSeq was introduced in Go 1.24 (February 2025). The code snippet at lines 544-547 does not show this function. Confirm whether strings.SplitSeq appears at lines 563-570 or elsewhere, and verify the project's minimum Go version in go.mod to determine if this is an actual compatibility concern.

pkg/annotations/v2/annotations.go (2)

82-87: Note: External annotations included for consistency.

OCISeccompBPFHook and StopSignal are external annotations (OCI/containers standards) rather than CRI-O-specific ones. Including them here centralizes annotation constants, but they don't follow the *.crio.io naming convention since they're defined by external specifications.

---

258-283: Annotations without V1 equivalents are V2-only.

AllAnnotations includes annotations like CPULoadBalancing, CPUQuota, CPUCStates, etc., that are not in reverseAnnotationMigrationMap. This is correct if these are new V2-only annotations without legacy equivalents.

internal/lib/container_server.go (1)

269-280: LGTM!

The migration to V2 annotation lookups is consistently applied throughout LoadSandbox and LoadContainer:

• PodLinuxOverhead, PodLinuxResources, UsernsMode, Spoofed, StopSignal, and PlatformRuntimePath all use v2.GetAnnotationValue for automatic V1 fallback
• Error messages correctly reference the V2 annotation keys
• The previous review comments about inconsistent StopSignal handling have been addressed

Also applies to: 292-294, 357-364, 535-555

internal/config/seccomp/seccompociartifact/seccompociartifact.go (2)

49-67: LGTM!

The use of GetAnnotationValueWithKey enhances logging by showing the actual annotation key that was matched (whether V1 or V2). The strings.TrimSuffix calls correctly extract the base key for clearer log messages regardless of which format was matched.

---

32-32: Consistent constant migration.

SeccompProfilePodAnnotation correctly uses the V2 base constant with the "/POD" suffix, maintaining the expected annotation pattern.

server/sandbox_run_linux.go (1)

28-28: LGTM! Clean migration to v2 annotations.

The file consistently migrates to the v2 annotation API by:

• Importing the v2 package with alias
• Using v2.GetAnnotationValue() for retrieving annotation values
• Referencing v2 constants (UsernsMode, ShmSize, StopSignal, PodLinuxOverhead, PodLinuxResources, Spoofed, LinkLogs)

The migration maintains backward compatibility through the v2 helper which automatically falls back to V1 annotations.

Also applies to: 40-40, 496-496, 1014-1014, 1074-1074, 1194-1194, 1204-1204, 1314-1314, 1430-1430

pkg/annotations/annotations.go (3)

9-19: LGTM! Good backward-compatible wrapper.

The GetAnnotationValue wrapper correctly delegates to v2.GetAnnotationValue() and includes clear deprecation guidance directing users to import v2 directly. This maintains backward compatibility while encouraging migration.

---

22-133: LGTM! Consistent deprecation of V1 constants.

All V1 annotation constants are properly:

• Mapped to their v2 equivalents (e.g., UsernsModeAnnotation = v2.V1UsernsMode)
• Documented with deprecation notices
• Guided to use v2 constants instead

This maintains backward compatibility while clearly communicating the migration path.

---

135-160: LGTM! Comprehensive annotation composition.

The AllAllowedAnnotations composition correctly includes:

1. External annotations (rdt, org.systemd.property, etc.)
2. V2 recommended annotations via v2.AllAnnotations
3. V1 deprecated annotations via v2.AllV1Annotations for backward compatibility

This ensures all annotations (new, old, and external) are recognized during the migration period.

contrib/test/ci/integration.yml (1)

65-65: LGTM! Consistent test skip pattern.

The addition of annotation_migration.bats to the kata skip list follows the established pattern for other test files and correctly references the kata skip variable defined in vars.yml.

README.md (1)

171-172: LGTM! Clear user-facing documentation.

The addition clearly informs users about the annotation migration and provides a direct link to the detailed migration guide. Good placement before the Security section.

ANNOTATION_MIGRATION.md (1)

1-228: LGTM! Comprehensive and well-structured migration guide.

This documentation provides excellent coverage of the annotation migration:

• Clear overview and rationale (following Kubernetes conventions)
• Complete migration mapping table for all 15 annotations
• Backward compatibility explanation with precedence rules
• Practical code examples for developers using v2.GetAnnotationValue()
• User-facing YAML examples showing old vs new formats
• Testing guidance
• Phased timeline approach

The document will effectively guide both users and developers through the migration.

server/container_create_freebsd.go (1)

22-22: LGTM! Consistent v2 annotation usage.

The changes correctly:

• Import the v2 package
• Use v2.GetAnnotationValue() to retrieve the devices annotation
• Reference the v2.Devices constant

The intermediate variable devicesAnnotationValue makes the code clear and maintains the existing error handling flow.

Also applies to: 212-213

contrib/test/ci/vars.yml (1)

180-186: LGTM! Comprehensive test coverage for annotation migration.

The kata skip configuration correctly defines tests that cover:

• V1 annotation backward compatibility (cgroup2-mount-hierarchy-rw, userns-mode, umask)
• V2 annotation functionality (cgroup2-mount-hierarchy-rw, userns-mode, unified-cgroup)
• Container-specific annotations with slash separator

This ensures both migration paths are tested while allowing kata environments to skip these tests appropriately.

internal/runtimehandlerhooks/high_performance_hooks_test.go (2)

25-25: LGTM! Correct import update.

The import correctly changes from pkg/annotations to pkg/annotations/v2 with the crioannotations alias, aligning with the v2 migration strategy.

---

428-428: LGTM! Consistent test updates to v2 constants.

All test code correctly updated to use v2 annotation constants:

• IRQLoadBalancingAnnotation → IRQLoadBalancing
• CPULoadBalancingAnnotation → CPULoadBalancing
• CPUSharedAnnotation → CPUShared

The changes are mechanical and maintain test functionality while aligning with the v2 annotation naming conventions.

Also applies to: 447-447, 455-455, 463-463, 471-471, 479-479, 1260-1260, 1492-1492, 1569-1569, 1576-1576, 1616-1616, 1637-1637, 1682-1682, 1687-1687

internal/runtimehandlerhooks/runtime_handler_hooks_linux.go (3)

9-9: LGTM: Import updated to v2 annotations package.

The import path has been correctly updated to use the v2 annotations package with an appropriate alias.

---

75-88: LGTM: High-performance annotations updated to v2.

The function correctly uses v2 annotation constants with strings.HasPrefix checks, which properly handles container-specific annotations that use the "/" separator pattern (e.g., cpu-shared.crio.io/containerName).

---

90-110: LGTM: CPU load balancing check updated to v2.

The function correctly uses the v2 CPULoadBalancing constant with direct equality checks for both runtime and workload allowed annotations.

server/container_create.go (7)

43-43: LGTM: Import updated to v2 annotations package.

The v2 annotations package is correctly imported with an appropriate alias.

---

682-683: LGTM: Cgroup2 mount annotation updated to v2.

The annotation retrieval correctly uses v2.GetAnnotationValue with the v2 constant Cgroup2MountHierarchyRW.

---

934-938: LGTM: Link logs annotation updated to v2.

The annotation retrieval correctly uses v2.GetAnnotationValue and properly checks for the annotation's presence before calling LinkContainerLogs.

---

984-989: LGTM: Disable FIPS annotation updated to v2.

The annotation retrieval correctly uses v2.GetAnnotationValue with the v2 constant. The double-check pattern (both ctr.DisableFips() and annotation value) ensures proper FIPS disabling.

---

1208-1210: LGTM: SELinux label skip annotation updated to v2.

The annotation retrieval correctly uses v2.GetAnnotationValue and properly checks both presence and value before setting the maybeRelabel flag.

---

1483-1500: LGTM: Umask annotation updated to v2.

The annotation retrieval correctly uses v2.GetAnnotationValue with the v2 constant Umask. The validation and parsing logic remains intact.

---

1453-1466: Verify the user namespace ID mapping logic.

This introduces conditional logic that prevents adding ID mappings when the sandbox has a user namespace (when sb.UserNsPath() != ""). The rationale is sound—OCI specs don't allow both a userns path and ID mappings simultaneously—but this is a behavioral change that requires verification against existing configurations and tests to ensure no regressions.

internal/runtimehandlerhooks/high_performance_hooks_linux.go (8)

29-29: LGTM: Import updated to v2 annotations package.

The import correctly references the v2 annotations package with an appropriate alias.

---

373-380: LGTM: CPU load balancing check updated to v2.

The function correctly uses the v2 CPULoadBalancing constant and maintains backward compatibility by accepting both "true" (deprecated) and "disable" values, with appropriate deprecation warnings.

---

382-389: LGTM: CPU quota check updated to v2.

The function correctly uses the v2 CPUQuota constant and follows the same pattern as shouldCPULoadBalancingBeDisabled with deprecation warnings and dual value support.

---

391-399: LGTM: IRQ load balancing check updated to v2.

The function correctly uses the v2 IRQLoadBalancing constant and extends the pattern to also accept "housekeeping" as a valid value alongside "true" and "disable".

---

401-405: LGTM: C-states configuration check updated to v2.

The function correctly uses the v2 CPUCStates constant and returns both the annotation value and presence indicator.

---

407-411: LGTM: Frequency governor check updated to v2.

The function correctly uses the v2 CPUFreqGovernor constant and follows the same pattern as shouldCStatesBeConfigured.

---

417-422: LGTM: Shared CPUs check updated to v2.

The function correctly uses the v2 CPUShared constant with the "/" separator for container-specific annotations, which aligns with the v2 annotation naming convention (e.g., cpu-shared.crio.io/containerName).

---

1519-1522: LGTM: Housekeeping CPUs check updated to v2.

The function correctly uses the v2 IRQLoadBalancing constant to check for the "housekeeping" value.