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

Image-installer: Fix duplication of image prefix #2172

Merged
merged 3 commits into from
Jun 23, 2022

Conversation

fastiuk
Copy link
Contributor

@fastiuk fastiuk commented May 18, 2022

  • SONiC prefix shown twice when working branch
    contains "image-" in its name.

Signed-off-by: Yevhen Fastiuk yfastiuk@nvidia.com

Why I did it

SONiC prefix is shown twice when the working branch contains "image-" in its name.
Screenshot 2022-05-18 at 14 08 12

How I did it

Fixed sonic-installer's one bootloader wrapper to show correct version.
Screenshot 2022-05-18 at 14 08 33

How to verify it

Build image (the branch is important):

cd sonic-buildimage
git checkout -b dev-image-test
make configure PLATFORM=mellanox && make target/sonic-mellanox.bin

Run image on the switch and execute:

sudo sonic-installer list

Previous command output (if the output of a command-line utility has changed)

Screenshot 2022-05-18 at 14 08 12

New command output (if the output of a command-line utility has changed)

Screenshot 2022-05-18 at 14 08 33

@liat-grozovik
Copy link
Collaborator

@fastiuk please note that new infra was added to test sonic-installer on #2179. Suggest you will wait for this PR to get merged and then add test to have this part covered as well.

@liat-grozovik
Copy link
Collaborator

@stepanblyschak please review

@liat-grozovik
Copy link
Collaborator

@stepanblyschak i am un able to assign you as a reviewer so sending a reminder :-) thanks.
@fastiuk please add UT please so we can signoff.

* SONiC prefix shown twice when working branch
contains "image-" in its name.

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>
* Fix conversion of image name/folder to image id
that replaces all occurrences of that prefix
(image-).
Buggy behavior:
 Name: image-dev-SONiC-image-ec0cedd5 ->
 ID: SONiC-dev-SONiC-SONiC-ec0cedd5
Correct behavior:
 Name: image-dev-SONiC-image-ec0cedd5 ->
 ID: SONiC-dev-SONiC-image-ec0cedd5
* Fix reverse conversion of image id to image
folder/name that replaces all occurrences of that
prefix (SONiC-).
Buggy behavior:
 ID: SONiC-dev-SONiC-image-ec0cedd5 ->
 Name: image-dev-image-image-ec0cedd5
Correct behavior:
 ID: SONiC-dev-SONiC-image-ec0cedd5 ->
 Name: image-dev-SONiC-image-ec0cedd5
Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>
@fastiuk fastiuk force-pushed the fix-image-installer-prefix branch from 005d3ac to d0292c4 Compare June 19, 2022 23:56
@qiluo-msft qiluo-msft merged commit 2f6a547 into sonic-net:master Jun 23, 2022
@yxieca
Copy link
Contributor

yxieca commented Jun 24, 2022

Is this a regression? If yes, which change introduced regression?

What platform(s) are impacted?

I didn't see this issue with 202205 image, is this issue platform specific? if not, why is it needed for 202205?

@fastiuk
Copy link
Contributor Author

fastiuk commented Jun 24, 2022

Is this a regression?

@yxieca I think this is a zero-day issue. At least I investigated so from git history.

@yxieca
Copy link
Contributor

yxieca commented Jun 24, 2022

I think this is a zero-day issue. At least I investigated so from git history.

@fastiuk, I think you have some image that we built, like the ones used for factory default image. I don't observe this issue on these images. Nor did I see this issue before. I am not convinced it is a zero-day issue. I am wondering if this change would cause issue on platforms that wasn't subject to this issue before?

@yxieca
Copy link
Contributor

yxieca commented Jun 24, 2022

Discussed with Qi. This is a very specific corner case.

@yxieca
Copy link
Contributor

yxieca commented Jun 24, 2022

This is a good change. But I don't think we need to cherry-pick it to a feature branch. The corner case is very specific.

@fastiuk fastiuk deleted the fix-image-installer-prefix branch July 2, 2022 20:17
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