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.

Sign up for GitHub

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

Jump to bottom

Add zypper ref before installing cockpit #16135

Merged
merged 1 commit into from
Dec 22, 2022
Merged

Add zypper ref before installing cockpit #16135

merged 1 commit into from
Dec 22, 2022

Conversation

jlausuch
Copy link
Contributor

@jlausuch jlausuch commented Dec 21, 2022 •
edited
Loading

@jlausuch jlausuch marked this pull request as draft December 21, 2022 08:16
@jlausuch
Copy link
Contributor Author

@ggardet FYI, just trying an idea from @okurz to see if metadata refresh helps with this random installation issue.

okurz
okurz reviewed Dec 21, 2022
View reviewed changes
tests/microos/cockpit_service.pm Outdated
@@ -49,6 +49,7 @@ sub run {

if (@pkgs) {
record_info('TEST', 'Installing Cockpit\'s Modules...');
assert_script_run('zypper ref');
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to at least add a comment here that the explicit zypper ref might help to work around some refresh problems. If it helps then I would still regard it as a product regression and should be reported as such.

@jlausuch
Copy link
Contributor Author

I've run some VRs running only cockpit module and all are green. Maybe we can give it a shot and see how this behaves during next builds.
https://openqa.opensuse.org/tests/2968953
https://openqa.opensuse.org/tests/2968975
https://openqa.opensuse.org/tests/2968976
https://openqa.opensuse.org/tests/2968977
https://openqa.opensuse.org/tests/2968978

@jlausuch jlausuch marked this pull request as ready for review December 21, 2022 16:13
ggardet
ggardet approved these changes Dec 22, 2022
View reviewed changes
Copy link
Collaborator

@ggardet ggardet left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@jknphy
Copy link
Contributor

jknphy commented Dec 22, 2022 •
edited
Loading

could be needed to exclude for ALP same configuration that is excluded for JeOS aarch in previous step?
https://openqa.opensuse.org/tests/2956621/modules/libzypp_config/steps/1/src

@jlausuch
Copy link
Contributor Author

could be needed to exclude for ALP same configuration that is excluded for JeOS aarch in previous step? https://openqa.opensuse.org/tests/2956621/modules/libzypp_config/steps/1/src

Could you explain why? In any case, I guess that would be for a different PR.

@jlausuch
Copy link
Contributor Author

Merging. Let's see how this goes.

@jlausuch jlausuch merged commit 28c250e into os-autoinst:master Dec 22, 2022
@jknphy
Copy link
Contributor

jknphy commented Dec 22, 2022

could be needed to exclude for ALP same configuration that is excluded for JeOS aarch in previous step? https://openqa.opensuse.org/tests/2956621/modules/libzypp_config/steps/1/src

Could you explain why? In any case, I guess that would be for a different PR.

it was just a grep, wrong clue, just looking at previous steps related with zypper, I'm wondering if we should do zypper ref after enable the repo at the very beginning, but yes, for another PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@okurz okurz okurz left review comments

@grisu48 grisu48 grisu48 approved these changes

@ggardet ggardet ggardet approved these changes

@foursixnine foursixnine Awaiting requested review from foursixnine

@jknphy jknphy Awaiting requested review from jknphy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@jlausuch @jknphy @grisu48 @okurz @ggardet