Skip to content
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

Add annotations for creating OCI images from checkpoint archives #127

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

Parthiba-Hazra
Copy link
Contributor

These annotations provide metadata including the container manager, container-name, pod, namespace for creating OCI images from checkpoint archives.

Copy link

github-actions bot commented Apr 15, 2024

Test Results

55 tests  ±0   55 ✅ ±0   1s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 1c47bb2. ± Comparison against base commit 873b71c.

♻️ This comment has been updated with latest results.

@Parthiba-Hazra
Copy link
Contributor Author

@rst0git @adrianreber Let's first confirm the structure of the annotation. After that, I will add additional annotations for Podman and CRI-O.

@adrianreber
Copy link
Member

This feels redundant: checkpointctl.checkpoint. Let's remove one of those words. I think checkpointctl is not really necessary. Even if we said so earlier.

@rst0git
Copy link
Member

rst0git commented Apr 15, 2024

@adrianreber How should we handle backward comparability with CRI-O and Podman? Do we want to deprecate the old annotations and provide support for them up to specified version, or to simply replace the annotations with a "breaking change".

@adrianreber
Copy link
Member

@adrianreber How should we handle backward comparability with CRI-O and Podman? Do we want to deprecate the old annotations and provide support for them up to specified version, or to simply replace the annotations with a "breaking change".

For CRI-O it is relatively easy as we only use one annotation. We will just handle both (for some time or forever). For Podman I would personally do the same. No need to break anything for the annotations used by Podman. For the ones not used I would just rename them.

lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
lib/annotations.go Outdated Show resolved Hide resolved
@Parthiba-Hazra Parthiba-Hazra force-pushed the annotations branch 2 times, most recently from bea3477 to 1ea9849 Compare April 25, 2024 17:37
@Parthiba-Hazra Parthiba-Hazra marked this pull request as ready for review April 25, 2024 17:38
@rst0git
Copy link
Member

rst0git commented May 29, 2024

@adrianreber PTAL

lib/annotations.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.73%. Comparing base (6c3a263) to head (27b455e).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #127   +/-   ##
=======================================
  Coverage   78.73%   78.73%           
=======================================
  Files          11       11           
  Lines        1260     1260           
=======================================
  Hits          992      992           
  Misses        201      201           
  Partials       67       67           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@adrianreber
Copy link
Member

The test failure is unexpected. Seems like some project changed the minimum Go version without changing its version. Strange.

I guess we need another PR doing make vendor before we can merge this one.

@Parthiba-Hazra
Copy link
Contributor Author

The test failure is unexpected. Seems like some project changed the minimum Go version without changing its version. Strange.

I guess we need another PR doing make vendor before we can merge this one.

@adrianreber I opened one here #133

@adrianreber
Copy link
Member

Please rebase.

- These annotations provide metadata including the container
  manager, container-name, pod, namespace for creating OCI
  images from checkpoint archives.

Signed-off-by: Parthiba-Hazra <[email protected]>
@rst0git rst0git merged commit 394ce4e into checkpoint-restore:main Jun 10, 2024
10 checks passed
rst0git added a commit to rst0git/checkpointctl that referenced this pull request Jun 10, 2024
Major highlights of this release:

- Added annotations for creating OCI images (checkpoint-restore#127)
- Added support for list command (checkpoint-restore#115)
- Added auto-completion files (checkpoint-restore#118)

Signed-off-by: Radostin Stoyanov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants