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

MicroOS: Add basic tests for Toolbox #12038

Merged
merged 1 commit into from
Mar 16, 2021
Merged

MicroOS: Add basic tests for Toolbox #12038

merged 1 commit into from
Mar 16, 2021

Conversation

jlausuch
Copy link
Contributor

@jlausuch jlausuch commented Feb 26, 2021

They are applicable for SLE Micro and openSUSE MicroOS

@jlausuch jlausuch requested a review from Vogtinator February 26, 2021 14:46
@jlausuch
Copy link
Contributor Author

@dfaggioli @thkukuk FYI

@asmorodskyi
Copy link
Member

LGTM .
Does it make sense to do also run this with docker ?

@jlausuch
Copy link
Contributor Author

LGTM .
Does it make sense to do also run this with docker ?

Thanks. No, docker is not part of MicroOS/SLE Micro

my $image = is_sle_micro ? 'opensuse/toolbox:latest' : 'suse/sle-micro/5.0/toolbox:latest';
my $expected = is_sle_micro ? 'opensuse' : 'sles';
assert_script_run 'echo -e "REGISTRY=' . $registry . '\nIMAGE=' . $image . '" > ~/.toolboxrc';
validate_script_output 'toolbox cat /etc/os-release', sub { m/${expected}/ }, timeout => 180;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not check_os_release($expected, 'PRETTY_NAME', 'toolbox'); would it work?

Copy link
Contributor Author

@jlausuch jlausuch Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tried and it doesn't work, cause it does toolbox grep -e '^PRETTY_NAME\b' /etc/os-release
and it complains toolbox: invalid option -- 'e'.
So, we would need to tweak the function to achieve issuing toolbox -- grep -e '^PRETTY_NAME\b' /etc/os-release, but I don't think it's needed for this single thing.

Copy link
Member

@Vogtinator Vogtinator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI fails, just some minor comments.

tests/microos/toolbox.pm Outdated Show resolved Hide resolved
tests/microos/toolbox.pm Outdated Show resolved Hide resolved
tests/microos/toolbox.pm Show resolved Hide resolved
tests/microos/toolbox.pm Outdated Show resolved Hide resolved
@jlausuch
Copy link
Contributor Author

jlausuch commented Mar 2, 2021

SLE Micro

For TW, I am still facing some issues that are already solved in toolbox repo, but are not pushed to the image yet.

@jlausuch
Copy link
Contributor Author

jlausuch commented Mar 2, 2021

I am ready with the PR but I would like to wait until getting latest toolbox fixes in TW. Currently failing in some bug already fixed in toolbox: http://fromm.arch.suse.de/tests/293#step/toolbox/46
Will move to ready when I do a successful VR in TW. SLE looks fine.

@jlausuch
Copy link
Contributor Author

jlausuch commented Mar 5, 2021

Latest code works on SLE Micro but fails in TW due to this issue: openSUSE/microos-toolbox#25

Will remove notready label when the problem is fixed.

@jlausuch jlausuch added the Ready Ready for review label Mar 14, 2021
@jlausuch
Copy link
Contributor Author

The problem is already fixed in TW.
Verification runs:

@jlausuch jlausuch removed the notready label Mar 14, 2021
record_info('ISSUE', 'https://github.com/kubic-project/microos-toolbox/issues/23');
}
script_run 'toolbox run -c devel -- zypper lr'; # this command will fail in SLE Micro toolbox as there are no repos
assert_script_run 'toolbox run -c devel -- zypper -n in python3' unless is_sle_micro;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you testing zypper here ? than I would go for something less error prone and more stable to not dive into all surprises which python3 might bring ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just followed the same thing we do in container tests. But you are right, we could just tests something more meaningful for a toolbox use case (htop, nmap, ...)


our $user = 'test_user';

sub cleanup {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, I can call clean_container_host from here but I need to keep userdel -rf $user anyways.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 options possible and I don't have strong opinion which one is better here ... :

  1. leave things as it is right now .
  2. call cleanup from containers/common.pm here to avoid duplication with podman part of cleanup
  3. increase amount of if's in containers/common.pm and merge altogether

each choice has down and up sides so I am not sure ....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option 2 for now :)

They are applicable for SLE Micro and openSUSE MicroOS
Copy link
Member

@asmorodskyi asmorodskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jlausuch
Copy link
Contributor Author

Latest code VRs:microos sle-micro

@jlausuch
Copy link
Contributor Author

@Vogtinator this one is also ready, any other change requested?

@asmorodskyi asmorodskyi merged commit dc9cdd3 into os-autoinst:master Mar 16, 2021
@jlausuch jlausuch deleted the toolbox_test branch April 13, 2021 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants