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

Additional RPM repository support #471

Conversation

M4rtinK
Copy link
Contributor

@M4rtinK M4rtinK commented Jan 29, 2021

The first commit adds two library functions for working with additional RPM repositories for kickstart tests.

The second commit uses these library functions in the new Initial Setup reboot tests that need a latest Aanconda scratchbuild to work effectively.

@M4rtinK M4rtinK force-pushed the master-add_per_test_additional_repo_support branch from ae32193 to 371948c Compare January 29, 2021 14:56
@VladimirSlavik
Copy link
Contributor

Umm, how is this related to #469 ?

@M4rtinK
Copy link
Contributor Author

M4rtinK commented Jan 29, 2021

Umm, how is this related to #469 ?

That's a good question - this is specific to supporting the Initial Setup reboot tests while #469 is a generic "I want to add a bunch of packages to the RPM transaction" support. But we are discussion maybe scrapping that for now as we have not been able to come up with some pressing needs for doing that.

Also using the functionality added by this PR one could still quite easily add packages to a specific kickstart ad-hoc, just by copy-pasting the necessary lines from the Initial Setup reboot tests & putting the packages in data/additional_repo folder in kickstart tests checkout root.

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

I have some suggestions. Otherwise, it looks good to me. Thanks!

prepare() {
ks=$1
tmpdir=$2
export_additional_repo $tmpdir
Copy link
Contributor

Choose a reason for hiding this comment

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

You can probably call this in the kernel_args function. The prepare function is basically for preparing a kickstart.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! :) That simplifies things quite a bit - thanks! :)

tmpdir=$1

# cleanup the localhost webserver we might have started
if [ -f ${tmpdir}/httpd-pid ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

You can call the stop_httpd function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this I left the changes in initial-setup-gui.sh by mistake, I'll drop it from there and only leave it in the Initial Setup reboot tests as we discussed earlier.

}

cleanup() {
tmpdir=$1
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, use local to define local variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do. :)

functions.sh Show resolved Hide resolved
@M4rtinK M4rtinK force-pushed the master-add_per_test_additional_repo_support branch 2 times, most recently from e9cc25e to dba698d Compare February 3, 2021 13:34
@M4rtinK
Copy link
Contributor Author

M4rtinK commented Feb 3, 2021

PR updated:

  • applied feedback from @poncovka - thanks! :)
  • now dumps additional repo listing to file for easy debugging
  • all seems to work fine with additional RPMs added during local testing (the IS reboot tests both run to SUCCESS & from the log files its apparent the Anaconda scratchbuild passed via the additional repo is being used)

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

Add a couple library functions that make it easy to include additional
RPM repositories to a kickstart test.

By using these functions in it's sh script a kickstart tests can easily
add arbitrary RPM packages from the data folder to the installation
transaction. This can be useful to test a couple package scratchbuilds
without having to manipulate the full installation repos to do this.

The mechanism works by checking if the "additional_repo" folder exists
in the data folder. If it does a localhost HTTP server is started and
it's address is written to the addrepo_url file in the tests tempdir.

NOTE: We expect the additional_repo folder to just contain a bunch of
RPM files. The library function will make sure to run createrepo as
needed on it.

There is then a second library function that checks if the addrepo_url
file exists and adds appropriate inst.addrepo invocation to the passed
boot option script.

But functions still consider the additional repo to be optional,
so if no additional_repo exists in the data folder, they will be
effectively a no-op and the kickstart test using them will continue to
execute as usual, of course without the additional packages being added.
Use the library functions that enable access to additional RPM
repos in the Initial Setup reboot tests.

These tests aim is to verify that latest Anaconda changes have not
broken Initial Setup. To do this they need an Anaconda scratchbuild with
the latest changes (usually still in PR form) to be injected to the
package installation transaction as otherwise the stable released
package from the main repo would be installed and no breakage could
be uncovered.
@M4rtinK M4rtinK force-pushed the master-add_per_test_additional_repo_support branch from dba698d to 8d3ebf9 Compare February 4, 2021 11:58
@M4rtinK M4rtinK merged commit d6d8390 into rhinstaller:master Feb 4, 2021
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