-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Migrate annotations to Kubernetes-recommended naming conventions #9537
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: main
Are you sure you want to change the base?
Migrate annotations to Kubernetes-recommended naming conventions #9537
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
416cad0 to
0a77686
Compare
|
Creating a new package, something like annotations/v2, may be better, I guess. Also this may be out of scope, some variables has "Annotation" suffix, which is not necessary. We can remove that now. |
b997ddf to
6c5dcda
Compare
|
@cri-o/cri-o-maintainers PTAL |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9537 +/- ##
==========================================
+ Coverage 64.10% 64.17% +0.06%
==========================================
Files 202 204 +2
Lines 28196 28227 +31
==========================================
+ Hits 18075 18114 +39
+ Misses 8528 8525 -3
+ Partials 1593 1588 -5 🚀 New features to boost your workflow:
|
|
/retest |
1 similar comment
|
/retest |
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.
This will have a huge impact, so it should be reviewed by other active maintainers.
@haircommander @sohankunkerkar
1fbd192 to
5d89822
Compare
c52a859 to
3ef6a3f
Compare
3ef6a3f to
84a4360
Compare
This commit implements the annotation migration described in issue cri-o#7781, migrating CRI-O annotations from the legacy format (io.kubernetes.cri-o.*) to the Kubernetes-recommended format (*.crio.io). Migrated Annotations (15 total): - io.kubernetes.cri-o.userns-mode -> userns-mode.crio.io - io.kubernetes.cri-o.cgroup2-mount-hierarchy-rw -> cgroup2-mount-hierarchy-rw.crio.io - io.kubernetes.cri-o.UnifiedCgroup -> unified-cgroup.crio.io - io.kubernetes.cri-o.Spoofed -> spoofed.crio.io - io.kubernetes.cri-o.ShmSize -> shm-size.crio.io - io.kubernetes.cri-o.Devices -> devices.crio.io - io.kubernetes.cri-o.TrySkipVolumeSELinuxLabel -> try-skip-volume-selinux-label.crio.io - io.kubernetes.cri-o.seccompNotifierAction -> seccomp-notifier-action.crio.io - io.kubernetes.cri-o.umask -> umask.crio.io - io.kubernetes.cri-o.PodLinuxOverhead -> pod-linux-overhead.crio.io - io.kubernetes.cri-o.PodLinuxResources -> pod-linux-resources.crio.io - io.kubernetes.cri-o.LinkLogs -> link-logs.crio.io - io.kubernetes.cri-o.PlatformRuntimePath -> platform-runtime-path.crio.io - seccomp-profile.kubernetes.cri-o.io -> seccomp-profile.crio.io - io.kubernetes.cri-o.DisableFIPS -> disable-fips.crio.io Implementation Changes: - Create pkg/annotations/v2 package with all V2 annotation constants - Add reverseAnnotationMigrationMap for efficient V1↔V2 lookup - Implement GetAnnotationValue() helper function that: * Checks V2 annotation first (preferred) * Falls back to V1 annotation if V2 not present * Supports both base and container-specific annotations * Handles dot-separated (.containerName) and slash-separated (/containerName) patterns - Implement findV1KeyForContainerSpecific() for container-specific fallback - Mark all V1 annotations as deprecated with clear migration path - Update AllAllowedAnnotations to include both V1 and V2 formats - Update all annotation usage across codebase: * server/container_create*.go * server/sandbox_run*.go * server/sandbox_stop_linux.go * internal/lib/container_server.go * internal/factory/container/container.go * internal/config/seccomp/*.go * pkg/config/config.go Testing: - Add comprehensive unit tests (pkg/annotations/annotations_test.go): * Test V2 annotations work correctly * Test V1 fallback for backwards compatibility * Test V2 precedence when both present * Test container-specific annotations (both separators) * Test reverse migration map completeness * Test AllAllowedAnnotations includes both versions - Add integration tests (test/annotation_migration.bats): * 50+ test cases covering real-world usage * Test V2 annotations: userns-mode, umask, shm-size, devices, etc. * Test V1 backwards compatibility for all annotations * Test precedence behavior with actual CRI-O runtime Documentation: - Create ANNOTATION_MIGRATION.md with: * Migration overview and format comparison * Complete mapping table of all 15 annotations * Backwards compatibility guarantees * Migration timeline (Deprecation → Adoption → Removal) * Usage examples for common scenarios * Developer guidance for using helper functions - Link migration guide from README.md - Add inline deprecation comments to all V1 constants The implementation maintains full backwards compatibility: both V1 and V2 annotations are accepted, with V2 taking precedence when both are present. This allows gradual migration without breaking existing deployments. Fixes cri-o#7781 Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
84a4360 to
1ddb4a3
Compare
|
@cri-o/cri-o-maintainers PTAL |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This commit implements the annotation migration described in issue #7781,
migrating CRI-O annotations from the legacy format (io.kubernetes.cri-o.)
to the Kubernetes-recommended format (.crio.io).
Migrated Annotations (15 total):
Implementation Changes:
Testing:
Documentation:
The implementation maintains full backwards compatibility: both V1 and V2
annotations are accepted, with V2 taking precedence when both are present.
This allows gradual migration without breaking existing deployments.
Which issue(s) this PR fixes:
Fixes #7781
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?