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

manage_package_repo is gone and replacement is broken #250

Closed
KlavsKlavsen opened this issue Jun 29, 2018 · 9 comments · Fixed by #254
Closed

manage_package_repo is gone and replacement is broken #250

KlavsKlavsen opened this issue Jun 29, 2018 · 9 comments · Fixed by #254

Comments

@KlavsKlavsen
Copy link

When you removed manage_package_repo - a doc was added 517469b#diff-04c6e90faac2675aa89e2176d2eec7d8R188 - stating how to use the new.
It does however NOT work.
I made it work by adding:
Optional[String] $package_name = undef,
Boolean $manage_omnibus_repository = true,
to gitlab class - and make package::install - default to using $gitlab::package_name and obmibus_.. class - to using $gitlab::manage_ominibus_repository

Otherwise - one cannot use gitlab - and have repos handled by other modules.

@LongLiveCHIEF
Copy link
Contributor

What version of this package are you using?

This has not yet been released, and as such is unstable. I don't recommend using master for your installation. Use the latest stable release by viewing the releases page, and use the docs in the README.md for the latest release.

@KlavsKlavsen
Copy link
Author

I know.. I just checked out master and saw it was broken (for how I use it atleast :) I have kept my production gitlab version at an older release for now :)

@LongLiveCHIEF
Copy link
Contributor

I'm going to be adding some more tests against the new functionality added before I release to make sure any issues are addressed. It will be a breaking change though, so keep an eye out for things that will be different.

@LongLiveCHIEF
Copy link
Contributor

@KlavsKlavsen I'm working through what tests I need to add to this, but I need some help with the usage case you're trying to explain.

The module is designed to fail when you set manage_upstream_edition => 'disabled' and don't set package_name for gitlab::install.

What I'm trying to do is reduce the footprint of the docs in manifests/init.pp by factoring out things that are rarely used, or aren't directly related to the omnibus package, into the classes that configure them.

I can go back to the scoped method you describe, but I'd like to see if this new way can work first. I'm going to be doing a series/book on puppet development patterns in Puppet 5+, and this is one of the patterns I'm exploring.

So to be clear, I'm not saying you're wrong about anything... I think there probably is something I missed, I just need you to take another stab at explaining it.

Can you tell me what exactly you mean by:

Otherwise - one cannot use gitlab - and have repos handled by other modules.

Can you give me an example use case so I can add that as a test and scenario for a fix?

@KlavsKlavsen
Copy link
Author

an example use case would be to add a gitlab-ce repo (from other class) pointing to local mirror - and have NO internet connection. Thats our issue.

@LongLiveCHIEF
Copy link
Contributor

LongLiveCHIEF commented Jul 9, 2018

I think what you're saying, is that when you have some puppet code almost exactly like the code below, it fails. Right? If so can you provide me with the error you're seeing, along with any differences in your params for the gitlab and gitlab::install configurations?

class my_gitlab_profile (
  $manage_upstream_edition = 'disabled',
  $ensure = '10.7.6-ce.0.el7',
  $package_name = 'gitlab-ce',
  $repository_configuration = {},
) {
  class { 'gitlab':
    manage_upstream_edition => $manage_upstream_edition
  }
  class {'gitlab::install':
    ensure => $ensure,
    package_name => $package_name,
  }
  class {'gitlab::omnibus_package_repository':
    repository_configuration => $repository_configuration,
  }

or if you're using hiera:

my_gitlab_profile::manage_upstream_edition: 'disabled'
my_gitlab_profile::ensure: '10.7.6-ce.0.el7'
my_gitlab_profile::package_name: 'gitlab-ce'
my_gitlab_profile::repository_configuration:
  yumrepo:
    gitlab_official_ce:
      baseurl: "https://my.mirror.com/repository/packages.gitlab.com/gitlab/gitlab-ce/el/7/$basearch"

edit: updated the example to add hiera configuration and common profile type parameterization

LongLiveCHIEF added a commit to LongLiveCHIEF/puppet-gitlab that referenced this issue Jul 9, 2018
@LongLiveCHIEF
Copy link
Contributor

@KlavsKlavsen i've went ahead and changed the way these variables are scoped up from the init manifest like normal. When I went to add the new class tests, I was getting the failures too, using the scenario above.

As a result, this should start working again with the new technique once #254 is merged. (all params are now on class {'gitlab': as you suggested.

@KlavsKlavsen
Copy link
Author

Perfect.. thank you very much.. I hope you could use the "early feedback" on the module :)

@LongLiveCHIEF
Copy link
Contributor

Definitely appreciated so i could address this before it made it to an official release. Thank you!

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 a pull request may close this issue.

2 participants