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

Refactor repository handling #815

Merged
merged 1 commit into from
Apr 24, 2020
Merged

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Mar 25, 2020

This removes the repo parameters from the main class, in favor of a standalone class that can be included. It uses an anchor because that can be collected in the main class to keep the correct dependency chaining while using composition.

Still a draft because I need to clean up some more things but now it's at least a backup.

@ekohl ekohl force-pushed the rewrite-repo-handling branch 3 times, most recently from 6a246c1 to 2cb5000 Compare April 21, 2020 22:04
@ekohl ekohl marked this pull request as ready for review April 22, 2020 14:57
@ekohl ekohl force-pushed the rewrite-repo-handling branch from 2cb5000 to efcebaa Compare April 22, 2020 14:57
configure_epel_repo => $configure_epel_repo,
configure_scl_repo => $configure_scl_repo,
if $configure_scl_repo {
package {'foreman-release-scl':
Copy link
Member

Choose a reason for hiding this comment

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

Was the intent to move this to centos-release-scl-rh instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. I first want to merge it this way and rely on it. I want to drop foreman-release-scl in a separate PR but with this merged, I can at least start to clean up acceptance tests in puppet-foreman_proxy which will help with EL8.

@ehelms
Copy link
Member

ehelms commented Apr 22, 2020

Gotcha. Does this need an associated installer migration to go with it?

@ekohl
Copy link
Member Author

ekohl commented Apr 22, 2020

I don't think so. We'll stop supporting managing the repositories since the parameters no longer exist, the installer should remove them. That said, it probably makes sense to track it in a Redmine issue which I'll create now.

@ekohl ekohl force-pushed the rewrite-repo-handling branch from efcebaa to bb1d3b0 Compare April 22, 2020 15:43
Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

Installation of packages like foreman-postgresql, rh-redis5-redis, and rh-ruby25-ruby is failing in the CentOS7 acceptance test.

Checking logs at https://travis-ci.org/github/theforeman/puppet-foreman/jobs/678229861 , I saw only these instances of foreman::repo:

  Notice: Compiled catalog for centos7-64.example.com in environment production in 0.18 seconds

  Info: Applying configuration version '1587570547'

  Notice: /Stage[main]/Main/Exec[Create certificate directory]/returns: executed successfully

  Notice: /Stage[main]/Main/Exec[Generate certificate]/returns: executed successfully

  Notice: /Stage[main]/Main/File[/etc/foreman-certs/key.pem]/mode: mode changed '0644' to '0640'

  Notice: /Stage[main]/Main/File[/etc/foreman-certs/certificate.pem]/mode: mode changed '0644' to '0640'

  Notice: /Stage[main]/Foreman::Repo/Foreman::Repos[foreman]/Foreman::Repos::Yum[foreman]/Yumrepo[foreman]/ensure: created

  Info: Yumrepo[foreman](provider=inifile): changing mode of /etc/yum.repos.d/foreman.repo from 600 to 644

  Notice: /Stage[main]/Foreman::Repo/Foreman::Repos[foreman]/Foreman::Repos::Yum[foreman]/Yumrepo[foreman-source]/ensure: created

  Info: Yumrepo[foreman-source](provider=inifile): changing mode of /etc/yum.repos.d/foreman-source.repo from 600 to 644

  Notice: /Stage[main]/Foreman::Repo/Foreman::Repos[foreman]/Foreman::Repos::Yum[foreman]/Yumrepo[foreman-plugins]/ensure: created

  Info: Yumrepo[foreman-plugins](provider=inifile): changing mode of /etc/yum.repos.d/foreman-plugins.repo from 600 to 644

  Notice: /Stage[main]/Foreman::Repo/Foreman::Repos[foreman]/Foreman::Repos::Yum[foreman]/Yumrepo[foreman-plugins-source]/ensure: created

  Info: Yumrepo[foreman-plugins-source](provider=inifile): changing mode of /etc/yum.repos.d/foreman-plugins-source.repo from 600 to 644

  Info: Creating state file /opt/puppetlabs/puppet/cache/state/state.yaml

  Notice: Applied catalog in 0.09 seconds

What is not clear to me is why foreman-release-scl didn't get installed.

@ekohl
Copy link
Member Author

ekohl commented Apr 22, 2020

Installation of packages like foreman-postgresql, rh-redis5-redis, and rh-ruby25-ruby is failing in the CentOS7 acceptance test.

Incorrect fact. Pushing an update.

@ekohl ekohl force-pushed the rewrite-repo-handling branch 2 times, most recently from b7789d4 to 03636d4 Compare April 23, 2020 08:24
@mmoll
Copy link
Contributor

mmoll commented Apr 24, 2020

tests fail 💔

This removes the repo parameters from the main class, in favor of a
standalone class that can be included. It uses an anchor because that
can be collected in the main class to keep the correct dependency
chaining while using composition.
@ekohl ekohl force-pushed the rewrite-repo-handling branch from 03636d4 to 43dbb3f Compare April 24, 2020 13:10
@mmoll mmoll merged commit 475afc0 into theforeman:master Apr 24, 2020
@mmoll
Copy link
Contributor

mmoll commented Apr 24, 2020

merged, bedankt @ekohl!

@ekohl ekohl deleted the rewrite-repo-handling branch May 1, 2020 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants