voxpupuli / puppet-gitlab

Puppet module to manage Gitlab (Omnibus)
https://forge.puppet.com/puppet/gitlab/
BSD 3-Clause "New" or "Revised" License
74 stars 164 forks source link

Add repository does not trigger apt-update #285

Closed gdowmont closed 2 years ago

gdowmont commented 5 years ago

In version 2.0 of the module apt update is triggered after repos are added but before the package is installed.

In 3.0.2 update is not being triggered and puppet apply fails on the first run as apt cannot find it in the repo.

baurmatt commented 5 years ago

Happens because of this change -> https://github.com/voxpupuli/puppet-gitlab/commit/95d38022cb05db4d4716d67f488429024db5a890#diff-40b04cbfcee72a712c6991a38e36b497

What would be needed is Apt::Source['gitlab_official_(ce|ee)'] -> Class['apt::update'] -> Package['gitlab-omnibus'] but I'm not sure what the appropriate place for this code would be. In my opinion the generalization of the repo management might be a little bit to much :)

gdowmont commented 5 years ago

Thanks, Matthias. I will give it a shot.

Since this was intended behaviour in the first place I am happy to close this issue.

baurmatt commented 5 years ago

Please don't close the issue, this is definitely something the module should handle. Perhaps @juniorsysadmin has an idea where this code snippet could be placed :)

juniorsysadmin commented 5 years ago

Similar entry in puppet-nodejs: https://github.com/voxpupuli/puppet-nodejs/pull/201

baurmatt commented 5 years ago

Yeah, but puppet-nodejs doesn't have the abstraction around the repository logic and uses clear classes for each operatingsystem. This isn't the case for puppet-gitlab. Of course you can just put something like the following in the gitlab::omnibus_package_repository class:

if $::osfamily == 'Debian' {
  Apt::Source['gitlab_official_(ce|ee)'] -> Class['apt::update'] -> Package['gitlab-omnibus']
}

Not sure if that's considered good style in a class which seems to be specifically abstracted into something which doesn't really care about the OS.

LongLiveCHIEF commented 5 years ago

We just need to add an apt update to the default settings for repo management for debian in the data/family/Debian.yaml file.

LongLiveCHIEF commented 5 years ago

I'll submit a patch later today.

Dan33l commented 5 years ago

@gdowmont thank you for this report.

How do you call / use the module ? I would like to understand why the acceptance tests of this module did not catch the issue.

Tests call module like this :

class { 'gitlab':
   external_url => "http://${::fqdn}",
}
gdowmont commented 5 years ago

I had to revert to the previous release for now but the config looked similar to this:


# == Class: custom_gitlab:package
#
# A wrapper class around the  module to allow for installation
# and configuration of gitlab
#
class custom_gitlab::package(

  $postgres_user = undef,
  $postgres_password = undef,
  $ses_user = undef,
  $ses_password = undef,
  $gitlab_shell_secret_token = undef,
  $gitlab_workhorse_secret_token = undef,
  $gitlab_otp_key_base = undef,
  $gitlab_rails_secret_token = undef,
  $gitlab_ci_secret_key_base = undef,
  $gitlab_ci_db_key_base = undef

) {

  class { 'gitlab':
    manage_upstream_edition => 'ee',
    package_ensure    => '11.4.3-ee.0',
    external_url      => 'https://<URL>/',
    gitlab_ci         => {
      'builds_directory' => '/efs/gitlab/builds',
      'secret_key_base'  => $gitlab_ci_secret_key_base,
      'db_key_base'      => $gitlab_ci_db_key_base
    },
    git_data_dirs => {
      default => {
        path => '/efs/gitlab/git-data'
      }
    },
    shell             => {
      'secret_token' => $gitlab_shell_secret_token,
    },
    gitlab_workhorse             => {
      'secret_token' => $gitlab_workhorse_secret_token,
    },
    gitlab_rails      => {
      'db_adapter'                     => 'postgresql',
      'db_encoding'                    => 'utf8',
      'db_host'                        => '<DB_URL>',
      'db_port'                        => '<DB_PORT>',
      'db_username'                    => '<user>',
      'db_password'                    => $postgres_password,
      'db_database'                    => 'gitlab',
      'db_key_base'                    => $gitlab_ci_db_key_base,
      'otp_key_base'                   => $gitlab_otp_key_base,
      'gitlab_email_enabled'           => true,
      'gitlab_email_display_name'      => 'Gitlab',
      'gitlab_email_reply_to'          => 'noreply@email',
      'gitlab_email_from'              => 'gitlab@email',
      'redis_host'                     => '<ELASTICACHE_URL>',
      'redis_port'                     => '6379',
      'shared_path'                    => '/efs/gitlab/shared',
      'uploads_directory'              => '/efs/gitlab/uploads',
      'secret_token'                   => $gitlab_rails_secret_token,
      'smtp_enable'                    => true,
      'smtp_address'                   => '<SMTP_URL>',
      'smtp_port'                      => '<SMTP_PORT>',
      'smtp_user_name'                 => $ses_user,
      'smtp_password'                  => $ses_password,
      'smtp_domain'                    => '<SMTP_DOMAIN>',
      'smtp_authentication'            => 'login',
      'smtp_enable_starttls_auto'      => true,
      'time_zone'                      => 'UTC',
      'backup_path'                    => '/efs/gitlab/backups',
      'backup_upload_connection'       => {
        'provider'        => 'AWS',
        'region'          => 'eu-west-1',
        'use_iam_profile' => true
      },
      'backup_upload_remote_directory' => '<REMOTE_DIRECTORY>',
      'backup_keep_time'               => '604800',
      'monitoring_whitelist'           => ['127.0.0.0/8', '192.168.0.1', '172.18.0.0/22']
    },
    high_availability => {
      'mountpoint' => '/efs'
    },
    nginx             => {
      'listen_port'       => '80',
      'listen_https'      => false,
      'proxy_set_headers' => {
        'X-Forwarded-Proto' => 'https',
        'X-Forwarded-Ssl'   => 'on'
      }
    },
    postgresql        => {
      'enable' => false
    },
    redis             => {
      'enable' => false
    },
    user              => {
      'username' => 'git',
      'group'    => 'git',
      'uid'      => 697,
      'gid'      => 698,
      'home'     => '/efs/gitlab/git-home',
      'shel'     => '/bin/bash'
    },
    web_server        => {
      'username' => 'gitlab-www',
      'group'    => 'gitlab-www',
      'uid'      => 698,
      'gid'      => 699,
      'home'     => '/var/opt/gitlab/nginx',
      'shell'    => '/bin/false'
    },
    gitaly                   =>{
      'ruby_num_workers' => 4
    },
  }

}```
Dan33l commented 5 years ago

During acceptance tests an apt-get update is triggered to be able to install puppet deb package via run_puppet_install_helper.

So i suppose that we have to update unit tests to ensure that update is present in catalog.

@LongLiveCHIEF if you agree, can you take care of this in the PR you planed ?

LongLiveCHIEF commented 5 years ago

We didn't have working acceptance test framework until just recently, which is why this didn't come up.

I will update tests as needed to cover this along with my patch.

LongLiveCHIEF commented 5 years ago

@gdowmont what version of puppetlabs/apt are you using? (Specifically is it 2.4 or later?)

In version 2.4 and later of puppetlabs/apt, apt::update gets called each time a config file is changed or added. https://forge.puppet.com/puppetlabs/apt#update-the-list-of-packages

You can see that the notify_update param is non-optional for the apt::source Class, and if not specified, will be set to true.

gdowmont commented 5 years ago

@LongLiveCHIEF It is later than 2.4. Excerpt from metadata.json

    {
      "name": "puppetlabs/apt",
      "version_requirement": ">= 4.5.1 < 5.0.0"
    }
LongLiveCHIEF commented 5 years ago

I think @baurmatt's solution would be the best.

I also want to be sure it will actually solve @gdowmont's issue. This abstraction pattern was implemented specifically to allow any version of apt to be supported, due to the common controlrepo pattern making it difficult for organizations to update that type of common module frequently. If @gdowmont is using puppetlabs/apt < 2.4 then I think it's confirmed that this would fix the issue.

If he's using puppetlabs/apt >= 2.4 then it's possible there's another problem with his configuration of apt at a higher level, and the patch won't fix it.

LongLiveCHIEF commented 5 years ago

It is later than 2.4. Excerpt from metadata.json

I know... I wrote it. :smiley:

Can you confirm that's what is on your system though? Most server/agent infrastructures completely ignore metadata.json files and install modules like apt into the basemodulepath based on whatever version your puppet infrastructure owners choose.

gdowmont commented 5 years ago

😄

Definitely got > 2.4. 4.5.1 to be specific

name": "puppetlabs-apt",
  "version": "4.5.1",
LongLiveCHIEF commented 5 years ago

@gdowmont can you confirm that apt::update is being run at all? The apt::source auto-triggers a refresh of the exec['apt_update'], so I'm trying to see if this is an ordering problem, or a symptom of a default frequency setting for apt::update from wherever it's being included in your system.

If everything installs correctly after running the puppet agent twice in a row, it's an ordering problem. If you run puppet twice in a row and it still doesn't update, then I don't think a patch will actually solve your issue.

LongLiveCHIEF commented 5 years ago

The tests I've written so far pass, because of this auto-execution that is triggered by the notify_update default parameter of apt::source.

gdowmont commented 5 years ago

@LongLiveCHIEF Running puppet twice installs gitlab correctly. As you said, it could be ordering problem.

LongLiveCHIEF commented 5 years ago

That's good. That's the result i was expecting.

The init.pp uses include gitlab::omnibus_package_repository instead of contain. I think I had to do this because using contain was causing dependency cycle errors.

If it were able to be contained, that would mean that the exec['apt_update'] would be run before the omnibus package resource with no other changes to the module needed.

I'll get crackin ASSAP. Thanks for the additional info @gdowmont .

LongLiveCHIEF commented 5 years ago

Now that #294 is merged I should be able to fix this to work as originally intended.

gdowmont commented 3 years ago

Just tried the latest version of the plugin for a new deployment and this issue is still present. I have noticed that there is a PR https://github.com/voxpupuli/puppet-gitlab/pull/388 open that might fix it. Is this something you can look at?