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

Add repository does not trigger apt-update #285

Closed
gdowmont opened this issue Oct 25, 2018 · 22 comments · Fixed by #388
Closed

Add repository does not trigger apt-update #285

gdowmont opened this issue Oct 25, 2018 · 22 comments · Fixed by #388
Assignees

Comments

@gdowmont
Copy link

gdowmont commented Oct 25, 2018

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
Copy link
Contributor

Happens because of this change -> 95d3802#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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Member

juniorsysadmin commented Nov 4, 2018

Similar entry in puppet-nodejs:
voxpupuli/puppet-nodejs#201

@baurmatt
Copy link
Contributor

baurmatt commented Nov 5, 2018

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
Copy link
Contributor

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
Copy link
Contributor

I'll submit a patch later today.

@Dan33l
Copy link
Member

Dan33l commented Nov 6, 2018

@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
Copy link
Author

gdowmont commented Nov 7, 2018

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
Copy link
Member

Dan33l commented Nov 7, 2018

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Author

gdowmont commented Nov 7, 2018

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

    {
      "name": "puppetlabs/apt",
      "version_requirement": ">= 4.5.1 < 5.0.0"
    }

@LongLiveCHIEF
Copy link
Contributor

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
Copy link
Contributor

It is later than 2.4. Excerpt from metadata.json

I know... I wrote it. 😃

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
Copy link
Author

gdowmont commented Nov 7, 2018

😄

Definitely got > 2.4.
4.5.1 to be specific

name": "puppetlabs-apt",
  "version": "4.5.1",

@LongLiveCHIEF LongLiveCHIEF self-assigned this Nov 7, 2018
@LongLiveCHIEF
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Author

gdowmont commented Nov 8, 2018

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

@LongLiveCHIEF
Copy link
Contributor

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
Copy link
Contributor

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

@gdowmont
Copy link
Author

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 #388 open that might fix it. Is this something you can look at?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants