Opened 5 years ago

Closed 3 years ago

#772 closed defect (fixed)

Configuration issue in Puppet setup for server5

Reported by: matze Assignee: matze
Priority: P4 Milestone:
Module: Infrastructure Keywords: externaldependency
Cc: trev, mathias@… Blocked By:
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://github.com/adblockplus/puppet-concat/pull/1

Description

When in development environment, both the initial vagrant up server5 as well as any subsequent vagrant provision server5 command currently leads to the following error:

warning: Scope(Concat[/home/rsync/.ssh/known_hosts]): Could not look up qualified variable 'concat::setup::root_group'; class concat::setup has not been evaluated at /tmp/vagrant-puppet-2/modules-0/concat/manifests/init.pp:129

It occurred with a fresh clone of the repository at aa17d808aee9, but seems to affect earlier versions, too. However, I'm not sure about the consequences in e.g. production yet - if any.

Change History (18)

comment:1 Changed 5 years ago by matze

  • Component changed from Unknown to Infrastructure
  • Owner set to matze

comment:2 Changed 5 years ago by matze

It seems like there are more servers affected:

  • filter1
  • filter2
  • filter3
  • filter4
  • filter5
  • filter6
  • filter7
  • filter8
  • notification1
  • notification2
  • server11
  • server12
  • server15
  • server19
  • server5
  • server6
  • server7

All of the aforementioned guests raise the exact same issue.

Last edited 5 years ago by matze (previous) (diff)

comment:3 Changed 5 years ago by matze

  • Blocking 760 added

comment:4 Changed 5 years ago by matze

Last edited 5 years ago by matze (previous) (diff)

comment:5 Changed 5 years ago by trev

  • Cc trev added
  • Priority changed from Unknown to P4
  • Ready set

There are no implications - this is a warning, not an error. Older versions of the concat module (like the one we are using) warned when concat::setup wasn't invoked explicitly, this has been fixed and concat::setup is now deprecated. Feel free to update concat, so far we didn't bother doing that.

comment:6 Changed 5 years ago by matze

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

I've prepared a pull request via GitHub (see review URL field above) for updating to the 1.0.4 version. I believe the hg Repository is just a clone of the former, so this seems the easiest way to go. Though it requires a subsequent merge from git to hg, however.

Currently this is still under test; I'll hard-wire it in my development environment and check whether the provisioning of each box is still working. Results will follow.

Fortunately this can be done with some commandline-foo, so it's more a background job. Yet it may take a while - my last run through all boxes (one initial and one additional provision) took around four hours.

comment:7 Changed 5 years ago by matze

The update has been applied via hg.adblockplus.org now. Testing is at 50%, and although no box so far seemed to have any trouble with the update, some of them still show the message quoted in the ticket description. I'll try to fix them in the context of #760.

comment:8 Changed 5 years ago by matze

  • Resolution set to fixed
  • Status changed from reviewing to closed

I'm closing this issue because I do not consider it an issue any more. Especially because it does not block #760. However, please note that the warnings won't disappear until I publish the changes for that very ticket.

comment:9 Changed 5 years ago by trev

It would probably be better to update the dependency on concat module in a separate commit referencing this issue rather than throwing it into #760.

comment:10 Changed 5 years ago by trev

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening since "fixed" is wrong regardless of what you decide to do, it's either "works for me" or "waiting for the fix."

comment:11 Changed 5 years ago by matze

Ok.

comment:12 Changed 5 years ago by matze

As discussed in IRC; I'll monitor the upstream (where the entire concat::setup class should disappear soon) for an update. IMHO, we can close this ticket now, especially because all remaining warnings are caused by the concat module itself and we do not plan to manually fix the module when the upstream is planning on doing to itself already.. OK?

comment:13 Changed 5 years ago by trev

  • Blocking 760 removed

comment:14 Changed 5 years ago by trev

I think the usual approach is setting externaldependency keyword and leaving the issue open. Does it make sense to update the dependency on concat module regardless or should we only do it once the issue is completely resolved?

comment:15 follow-up: Changed 5 years ago by matze

There is no need to do the update now, we could wait until they report the fix in their changelog. But:

An upgrade, whenever done, requires testing efforts. And this one I've tested excessively. Furthermore, it is always recommended do run updates and the like as often as you can (I don't think I need to examine the arguments for that here). So, since it does not seem to have any negative impact, why should we not upgrade now?

comment:16 Changed 5 years ago by matze

  • Cc mathias@… added
  • Keywords externaldependency added

comment:17 in reply to: ↑ 15 Changed 5 years ago by trev

Replying to matze:

So, since it does not seem to have any negative impact, why should we not upgrade now?

Feel free to commit a dependency update then, LGTM for that.

comment:18 Changed 3 years ago by ferris

  • Resolution set to fixed
  • Status changed from reopened to closed
  • Tester set to Unknown

concat::setup is not in use anymore.

Note: See TracTickets for help on using tickets.