voxpupuli / puppet-yum

Puppet module for Yum
https://forge.puppet.com/puppet/yum
MIT License
16 stars 101 forks source link

added CentOS 8 default repos #191

Closed Bonehead5338 closed 2 years ago

Bonehead5338 commented 4 years ago

Pull Request (PR) description

Add CentOS 8 default Repos

This Pull Request (PR) fixes the following issues

Fixes #176

Bonehead5338 commented 4 years ago

Those test failures look 'expected' because of the changes

Bonehead5338 commented 4 years ago

@bastelfreak is it worth me updating the test to make this pass i.e. would it be mergeable if I did?

bastelfreak commented 4 years ago

this looks good to me, but I'm not a RHEL expert. Will leave it open for a few days in case anybody else wants to review it.

jadestorm commented 3 years ago

Just to throw my 2 cents in =) this looks good to me. I'm going to be testing it shortly and will report back.

hboetes commented 3 years ago

A few things, after doing a puppet agent -t the

--- base.repo   2020-09-23 13:48:13.117303094 +0200
+++ base.repo.right 2020-09-23 13:47:47.513623632 +0200
@@ -1,6 +1,6 @@
 [base]
 name=CentOS-$releasever - Base
-mirrorlist=http://mirrorlist.centos.org/?release=$releasever&arch=$basearch&repo=os&infra=$infra
+mirrorlist=http://mirrorlist.centos.org/?release=$releasever&arch=$basearch&repo=BaseOs&infra=$infra
 enabled=1
 gpgcheck=1
 gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-8

This wrong repo= stems from:

# grep repo=os
data/os/RedHat/CentOS.yaml
11:    mirrorlist: 'http://mirrorlist.centos.org/?release=$releasever&arch=$basearch&repo=os&infra=$infra'
jadestorm commented 3 years ago

@hboetes Odd -- I'm not running into that issue unless it was updated after you posted this.

hboetes commented 3 years ago

Is that perhaps because you use hiera instead of puppet code? This is the puppet code that I use, which worked fine for Centos 6 and 7:

class { 'yum':
      keep_kernel_devel => false,
      clean_old_kernels => true,
      managed_repos     => ['base','updates','epel'],
}
jadestorm commented 3 years ago

That may very well be it! I am using this in hiera:

---
yum::managed_repos:
  - 'BaseOS'
  - 'AppStream'
  - 'extras'
  - 'PowerTools'
yum::repos:
  'BaseOS':
    enabled: true
  'AppStream':
    enabled: true
  'extras':
    enabled: true
  'PowerTools':
    enabled: true

Also note I'm not testing CentOS 6 or 7, just 8.

With that said -- it's working exactly as I expect! My only feedback is that centosplus seems to be missing (i only see the source one) and that some of the capitalization and names seem inconsistent with their upstream names. (extras vs Extras, stuff like that -- but not a big deal)

Bonehead5338 commented 3 years ago

I will have another look at it tonight or tomorrow, thanks for the feedback

igalic commented 3 years ago

@dave93cab ping

igalic commented 3 years ago

should we point the CentOS 6 repos @ their vault?

Bonehead5338 commented 3 years ago

@dave93cab ping

I did look at this and refactored it a bit, though I think it was working ok before. I couldn't remember how I had it set up to run tests locally. Not sure what that Debian test failure is. Maybe someone else can finish this off, any volunteers?

ekohl commented 3 years ago

I also ran into those fact issues @ https://github.com/voxpupuli/puppet-yum/pull/195#pullrequestreview-538721715. I'm certain they are due to differences in Facter 2/3 and Facter 4.

oniGino commented 2 years ago

what needs to be done to unblock this, this is a very old PR and still an issue, I've forked this repo to get around this in my environment. Please let me know

bastelfreak commented 2 years ago

@oniGino it needs to be rebased because there are merge conflicts.

oniGino commented 2 years ago

@oniGino it needs to be rebased because there are merge conflicts.

https://github.com/voxpupuli/puppet-yum/pull/225 I just filed its a rebased copy of this PR

it has passed all tests, please review.

This will also fix the following

224

176

132

seems this has been a blocker for allot of people for a while now, I hope we can get this merged quickly