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

Fix dhcp-relay and macsec dockers not being marked as local packages #13059

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

saiarcot895
Copy link
Contributor

@saiarcot895 saiarcot895 commented Dec 14, 2022

Signed-off-by: Saikrishna Arcot sarcot@microsoft.com

Why I did it

When INSTALL_DEBUG_TOOLS=y is set (i.e. debug tools and containers are enabled), the non-debug versions of the dhcp-relay and macsec dockers are not added into SONIC_PACKAGES_LOCAL, and thus the tagging for those packages is not handled correctly when loading those dockers.

This issue happens when a regular build is done, and then the debug image is built with INSTALL_DEBUG_TOOLS=y. When this happens, the build system tries to load the docker image from the target/. However, this image was tagged as if it is a local package, but now in this build, only the debug dockers are considered local package. Because of this, there's a mismatch in the expected tag name, where the loaded docker uses the image tag, but the build system uses latest tag instead.

Follow-up fix for #12959 and #12829.

How I did it

To fix this, change the code that checks to see if something is part of a local package by searching for both just the package name and with the -dbg suffix to the name.

How to verify it

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211

Description for the changelog

Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@saiarcot895
Copy link
Contributor Author

@stepanblyschak could you review this when you get a chance?

@xumia
Copy link
Collaborator

xumia commented Dec 15, 2022

How about always saving docker-dhcp-relay:latest to docker-dhcp-relay.gz?
A SONiC docker image does not have an individual version, it uses the image version. The image version varies in each of the commit.

For example:
Commit 1, SONiC_VERSION=version 1, do an image build, saving docker-dhcp-relay:<version 1>
Commit 2, do some changes not relative to docker-dhcp-relay, and build again, it will load the wrong version.

And we have different build options, the image tag should be static.

When INSTALL_DEBUG_TOOLS=y is set (i.e. debug tools and containers are
enabled), the non-debug versions of the dhcp-relay and macsec dockers
are not added into SONIC_PACKAGES_LOCAL, and thus the tagging for those
packages is not handled correctly when loading those dockers.

To fix this, change the code that checks to see if something is part of
a local package by searching for both just the package name and with the
-dbg suffix to the name.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
When saving the dockers to disk, always use the latest tag. The dockers
on disk might not necessarily be associated with a specified image,
since it may get reused for building a different image if the sources
haven't changed.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
This is mainly for vim syntax highlighting purposes.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
@saiarcot895 saiarcot895 force-pushed the fix-dhcp-relay-macsec-local branch from fc6bbb4 to ec68b8d Compare December 19, 2022 22:49
@saiarcot895
Copy link
Contributor Author

@xumia, that makes sense, dockers saved to disk can't have version-specific information.

I've modified my changes to save the docker with just the latest tag.

@stepanblyschak
Copy link
Collaborator

@xumia @saiarcot895 Yes, that makes sense. I will rework the fix for #11521

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.

3 participants