fix: unable to close, unable to unref last: unable to cleanup component version, while unref last: closing component version #1243
+8
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hilmarf
changed the title
hilmarf
changed the title
hilmarf
changed the title
hilmarf
changed the title
hilmarf
requested review from
Skarlso,
mandelsoft,
fabianburth and
jakobmoellerdev
Skarlso
reviewed
Jan 10, 2025
| @@ -128,6 +128,10 @@ func (c *componentAccessImpl) HasVersion(vers string) (bool, error) { | |||
| } | |||
|
|
|||
| func (c *componentAccessImpl) LookupVersion(version string) (*repocpi.ComponentVersionAccessInfo, error) { | |||
| // LookupVersion '0.0.1-20250108132333.build-af79499' would fail with: | |||
| // unable to unref last: unable to cleanup component version [%v] while unref last: closing component version [%v]: check failed: component version [%v] is invalid | |||
There was a problem hiding this comment.
Does the actual error contain [%v] instead of the version? Just curious. :D
There was a problem hiding this comment.
no - the actual error is:
closing session: unable to close: closing docker.io/recmo/docker-hello-tiny:0.0.1-20250108132333.build-af79499: unable to unref last: unable to cleanup component version docker.io/recmo/docker-hello-tiny/0.0.1-20250108132333.build-af79499 while unref last: closing component version docker.io/recmo/docker-hello-tiny:0.0.1-20250108132333.build-af79499: check failed: component version "0.0.1-20250108132333+af79499" is invalid
There was a problem hiding this comment.
The problem is, that a version with this .build- is not a valid version anymore for CVs. So, the error is basically correct, but it comes far too late.
It must be assured that the version name given at the API matches the version name stored in the component descriptor, otherwise the complete structure would be corrupted.
So, such a version should be rejected at all. Accepting two versions, the CV version and a technical versions used at the backend, as valid CV versions would be problematic. But there is no reason why to accept the technical version, also. (The ListVersions call correctly maps the technical versions found as OCI tag back to OCM versions).
So, there should be an error if the technical version is given, and it should be reported early.
May be you could add the following check to LookupVersion in api/ocm/cpi/repocpi/bridge_c.go Line 102
func (b *componentAccessBridge) LookupVersion(version string) (cpi.ComponentVersionAccess, error) {
i, err := b.impl.LookupVersion(version)
if err != nil {
return nil, err
}
if i == nil || i.Impl == nil {
return nil, errors.ErrInvalid("component implementation behaviour", "LookupVersion")
}
if i.Impl.GetDescriptor().Version != version {
i.Impl.Close()
return nil, errors.ErrInvalid("component version mismatch")
}
return NewComponentVersionAccess(b.GetName(), version, i.Impl, i.Lazy, i.Persistent, !compositionmodeattr.Get(b.GetContext()))
}
Skarlso
previously approved these changes
Jan 11, 2025
mandelsoft
requested changes
Jan 12, 2025
hilmarf
added
the
dev/second-opinion
which has been replaced by META_SEPARATOR to create OCI compliant tag
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it
When calling ocm-cli with oci-tag-version like
0.0.1-20250108132333.build-af79499e.g.It fails during the finalize step, when doing an update, where another round of version comparison happens.
Which issue(s) this PR fixes
Fixes: #1235
Related: #355