-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
quadlet: fix inter-dependency of containers in Network=
#24794
quadlet: fix inter-dependency of containers in Network=
#24794
Conversation
Ephemeral COPR build failed. @containers/packit-build please check. |
@Luap99 PTAL |
You have lint errors. |
LGTM |
5b511ec
to
a419adc
Compare
Sorry. Fixed. |
a419adc
to
ecf9b0a
Compare
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.
Tests are failing
Instead of addressing this only when needed, the way to address this issue is the same as we did for Line 584 in 07dddeb
|
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.
As mentioned, this is not the way to address this issue. I mark is as "Request Changes" to make sure it is not merged
Thanks for pointing this out. I'll rework this PR. |
Signed-off-by: Misaki Kasumi <[email protected]>
ecf9b0a
to
cf505fe
Compare
I have no idea why two tasks have failed. |
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.
LGTM
Not sure either, I've rerun the failed ones |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, ruihe774, ygalblum 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 |
This PR fixes a bug introduced by #23814.
Unlike network quadlet units, which are sorted before container quadlet units so that the
ResourceName
of which are guaranteed to be present when used inaddNetworks()
, there is no guaranteed resolving order between containers. As a result,ResourceName
of a container when used inaddNetworks()
may be empty because the container is not resolved (i.e.ConventContainer()
is not called for it) yet.This PR fixes this by adding special handling to resolve the resource name in
addNetworks()
for containers.@ygalblum PTAL