Add missing dependencies to allow repo initialization on first puppet run. #178

nuclearsandwich · 2018-01-12T00:32:54Z

Puppet, at least version 3.8.5, does not evaluate environment variable strings for variable expansion, which was leading to a PYTHONPATH variable of /home/jenkins-agent/reprepro-updater/src:$PYTHONPATH with a literal $PYTHONPATH.

This interfered with the default path and caused #174

I made a test deployment with this patch (and a few others). @gavanderhoorn if you have time to verify that this addresses #174 for you it would be appreciated.

Puppet, at least version 3.8.5, does not evaluate environment variable strings for variable expansion, which was leading to a PYTHONPATH variable of `/home/jenkins-agent/reprepro-updater/src:$PYTHONPATH` with a literal $PYTHONPATH. I think this is interfering with the default path and causing #174

jonazpiazu · 2018-01-12T09:15:33Z

modules/profile/manifests/ros/repo.pp

@@ -210,7 +210,7 @@
    exec {'init_main_repo':
      path        => '/bin:/usr/bin',
      command     => "/home/${agent_username}/reprepro-updater/scripts/setup_repo.py ubuntu_main -c",
-      environment => ["PYTHONPATH=/home/${agent_username}/reprepro-updater/src:\$PYTHONPATH"],
+      environment => ["PYTHONPATH=/home/${agent_username}/reprepro-updater/src"],
      user  => $agent_username,
      group  => $agent_username,
      unless      => "/home/${agent_username}/reprepro-updater/scripts/setup_repo.py ubuntu_testing -q",


There is a typo in line 216, it should be:

unless => "/home/${agent_username}/reprepro-updater/scripts/setup_repo.py ubuntu_main -q",

Note ubuntu_main vs ubuntu_testing.

Thanks for the catch!

jonazpiazu · 2018-01-12T09:21:33Z

The changes in the branch did not fix the problem in my case.

Sorry that I do not really know puppet, so I am not sure how to debug. I tried this naive approach:

    exec {'init_building_repo':
      path        => '/bin:/usr/bin',
      command     => "echo 1: \$PYTHONPATH >> /tmp/output.log",
      environment => ["PYTHONPATH=/home/${agent_username}/reprepro-updater/src"],
      user  => $agent_username,
      group  => $agent_username,
      unless     => "echo 2: \$PYTHONPATH >> /tmp/output.log; /home/${agent_username}/reprepro-updat
      logoutput   => on_failure,
      require     => [
        Vcsrepo["/home/${agent_username}/reprepro-updater"],
        File["/home/${agent_username}/.buildfarm/reprepro-updater.ini"],
      ]
    }

And that generated this output:

# cat /tmp/output.log 
2: /home/jenkins-agent/reprepro-updater/src
1: /home/jenkins-agent/reprepro-updater/src

So PYTHONPATH looks just fine, I am not sure why the error still happens.

gavanderhoorn · 2018-01-12T11:11:10Z

Seeing the comment by @jonazpiazu I haven't tested this yet @nuclearsandwich.

I do seem to remember that after sudo su - jenkins-agent I could not run the various scripts under reprepro-updater either after deployment finished (ie: the puppet run).

nuclearsandwich · 2018-01-12T18:14:13Z

Thanks for the feedback @jonazpiazu and @gavanderhoorn. I tore down and started on a fresh machine and it fails the first time but succeeds the second. I learned from @tfoote that puppet sometimes needs to be run twice (and observed this previously when provisioning master) but I don't remember it being necessary for repo hosts.

I think the change here is still warranted as there's not much use in having a literal $PYTHONPATH in the variable and leaving it in doesn't do what one might expect.

I'm going to dig through puppet to see if something is finishing the setup of python and should be required before the repository initializations.

While investigating #174 some typos were found in the repository initialization logic that were best resolved by a little puppet refactoring. Additionally, the exec and unless commands are now run with `python` rather than invoking the script directly as $0. To be honest, I do not know how these commands were executing without a crunchbang line. It's possible that this just hasn't worked as expected for some time. The decision to use python rather than python3 was made by examining how reprepro-updater is invoked by ros_buildfarm jobs.

gavanderhoorn · 2018-01-12T18:41:51Z

I'm by no means a puppet expert, but in my experience most problems are due to missing dependency declarations (or whatever puppet calls those). That is probably also the cause of #171 (I worked around that in #70 (comment)).

Running puppet twice is at best a work-around. It would be nice if we could deploy master, agent(s) and slave successfully, without needing that.

but I don't remember it being necessary for repo hosts.

Exactly. I wasn't expecting this either.

nuclearsandwich · 2018-01-12T21:06:08Z

Running puppet twice is at best a work-around. It would be nice if we could deploy master, agent(s) and slave successfully, without needing that.

Yep, I'm digging in to figure out what the missing link in the chain is here. Thanks for the input and patience.

I was under the impression that this module was builtin but I guess I was wrong.

Adding the repo_files variable to the previous File resource list was not sufficient.

nuclearsandwich · 2018-01-12T22:56:19Z

Okay, I think the latest changes should do it. I'm building new test branches and deploying to a new host to be sure.

tfoote · 2018-01-12T23:06:58Z

Before you do that you might want to remove the Class['python'], I think that's redundant with the module requirements.

nuclearsandwich · 2018-01-12T23:27:18Z

Before you do that you might want to remove the Class['python'], I think that's redundant with the module requirements.

Good catch. Because the require directive (rather than include) is used to bring in the profile::jenkins::agent class everything in there should be set up before anything within the profile::ros::repo class.

This is redundant at the moment as Python is required by profile::jenkins::agent. If the relationship between these classes ever changes this may need to be re-added.

nuclearsandwich · 2018-01-13T00:35:18Z

This will close #174 and together with #179 should yield a clean deployment for repo hosts.

gavanderhoorn · 2018-01-13T12:30:31Z

Just ran a repo deploy on a clean VM after merging this and #179 and so far it seems puppet has run to completion without any complaints.

This is just a test VM, so the repo host isn't being used for anything, but so far looks good.

Nice one @nuclearsandwich 👍.

gavanderhoorn

Seems to work. LGTM.

inigomartinez · 2018-01-15T12:05:33Z

I'm sorry to say that these fixes don't seem to work here. I have tried it several times by going back to a fresh imagen and executing reconfigure.bash master, and the result is always the same:

2018-01-15 12:55:40 +0100 /Stage[main]/Jenkins::Repo::Debian/Apt::Source[jenkins]/Apt::Setting[list-jenkins]/File[/etc/apt/sources.list.d/jenkins.list]/ensure
 (notice): created
2018-01-15 12:55:40 +0100 /Stage[main]/Jenkins::Repo::Debian/Apt::Source[jenkins]/Apt::Setting[list-jenkins]/File[/etc/apt/sources.list.d/jenkins.list] (info)
: Scheduling refresh of Class[Apt::Update]
2018-01-15 12:55:44 +0100 Puppet (err): Could not update: Execution of '/usr/bin/apt-get -q -y -o DPkg::Options::=--force-confold --force-yes install jenkins=
2.73.3' returned 100: Reading package lists...
Building dependency tree...
Reading state information...
E: Version '2.73.3' for 'jenkins' was not found
2018-01-15 12:55:44 +0100 /Stage[main]/Jenkins::Package/Package[jenkins]/ensure (err): change from purged to 2.73.3 failed: Could not update: Execution of '/u
sr/bin/apt-get -q -y -o DPkg::Options::=--force-confold --force-yes install jenkins=2.73.3' returned 100: Reading package lists...
Building dependency tree...
Reading state information...
E: Version '2.73.3' for 'jenkins' was not found
2018-01-15 12:55:44 +0100 /Group[jenkins] (notice): Dependency Package[jenkins] has failures: true
2018-01-15 12:55:44 +0100 /Group[jenkins] (warning): Skipping because of failed dependencies
2018-01-15 12:55:44 +0100 /User[jenkins] (notice): Dependency Package[jenkins] has failures: true
2018-01-15 12:55:44 +0100 /User[jenkins] (warning): Skipping because of failed dependencies
2018-01-15 12:55:44 +0100 /Stage[main]/Jenkins::Config/File[/var/lib/jenkins] (notice): Dependency Package[jenkins] has failures: true
2018-01-15 12:55:44 +0100 /Stage[main]/Jenkins::Config/File[/var/lib/jenkins] (warning): Skipping because of failed dependencies
2018-01-15 12:55:44 +0100 /Stage[main]/Jenkins::Config/File[/var/lib/jenkins/plugins] (notice): Dependency Package[jenkins] has failures: true
2018-01-15 12:55:44 +0100 /Stage[main]/Jenkins::Config/File[/var/lib/jenkins/plugins] (warning): Skipping because of failed dependencies
2018-01-15 12:55:44 +0100 /Stage[main]/Profile::Jenkins::Master/File[/var/lib/jenkins/credentials.xml] (notice): Dependency Package[jenkins] has failures: tru
e
2018-01-15 12:55:44 +0100 /Stage[main]/Profile::Jenkins::Master/File[/var/lib/jenkins/credentials.xml] (warning): Skipping because of failed dependencies

I tried this by setting BUILDFARM_DEPLOYMENT_BRANCH in buildfarm_depolyment_config to fix-repo-pythonpath.

gavanderhoorn · 2018-01-15T12:09:49Z

@inigomartinez: I'm not sure, but I believe this PR targets the repo deployment only. So master not working is expected at this point.

nuclearsandwich · 2018-01-16T17:43:25Z

I'm not sure, but I believe this PR targets the repo deployment only. So master not working is expected at this point.

This is correct. There are outstanding issues with master deployments (though if you run ./reconfigure master enough times it should get up and running 🙃) which I hope to address in more work later this week.

nuclearsandwich self-assigned this Jan 12, 2018

jonazpiazu reviewed Jan 12, 2018

View reviewed changes

Try adding python as a dependency of the repository initialization.

ee80cd5

nuclearsandwich added 3 commits January 12, 2018 13:19

Add python-configparser to repo init dependencies.

53bb12d

I was under the impression that this module was builtin but I guess I was wrong.

Add repo directories (/var/repos/{,ubuntu}) to init dependencies.

1d9beee

Require /var/repos correctly.

77134b5

Adding the repo_files variable to the previous File resource list was not sufficient.

nuclearsandwich changed the title ~~Remove unevaluated $PYTHONPATH from environment variable suffix.~~ Add missing dependencies to allow repo initialization on first puppet run. Jan 12, 2018

Remove redundant requirement.

cb5b5b5

This is redundant at the moment as Python is required by profile::jenkins::agent. If the relationship between these classes ever changes this may need to be re-added.

nuclearsandwich mentioned this pull request Jan 13, 2018

Create .gnupg directory for gpg.conf. #179

Merged

nuclearsandwich added buildfarm role: repo in review labels Jan 13, 2018

gavanderhoorn approved these changes Jan 13, 2018

View reviewed changes

nuclearsandwich merged commit bcc7f79 into master Jan 17, 2018

nuclearsandwich deleted the fix-repo-pythonpath branch January 17, 2018 00:37

nuclearsandwich mentioned this pull request Jan 18, 2018

repo deployment: init_building_repo: "ImportError: No module named configparser" #174

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing dependencies to allow repo initialization on first puppet run. #178

Add missing dependencies to allow repo initialization on first puppet run. #178

nuclearsandwich commented Jan 12, 2018 •

edited

Loading

jonazpiazu Jan 12, 2018

nuclearsandwich Jan 12, 2018

jonazpiazu commented Jan 12, 2018

gavanderhoorn commented Jan 12, 2018

nuclearsandwich commented Jan 12, 2018

gavanderhoorn commented Jan 12, 2018

nuclearsandwich commented Jan 12, 2018

nuclearsandwich commented Jan 12, 2018

tfoote commented Jan 12, 2018

nuclearsandwich commented Jan 12, 2018

nuclearsandwich commented Jan 13, 2018 •

edited

Loading

gavanderhoorn commented Jan 13, 2018

gavanderhoorn left a comment

inigomartinez commented Jan 15, 2018

gavanderhoorn commented Jan 15, 2018

nuclearsandwich commented Jan 16, 2018

Add missing dependencies to allow repo initialization on first puppet run. #178

Add missing dependencies to allow repo initialization on first puppet run. #178

Conversation

nuclearsandwich commented Jan 12, 2018 • edited Loading

jonazpiazu Jan 12, 2018

Choose a reason for hiding this comment

nuclearsandwich Jan 12, 2018

Choose a reason for hiding this comment

jonazpiazu commented Jan 12, 2018

gavanderhoorn commented Jan 12, 2018

nuclearsandwich commented Jan 12, 2018

gavanderhoorn commented Jan 12, 2018

nuclearsandwich commented Jan 12, 2018

nuclearsandwich commented Jan 12, 2018

tfoote commented Jan 12, 2018

nuclearsandwich commented Jan 12, 2018

nuclearsandwich commented Jan 13, 2018 • edited Loading

gavanderhoorn commented Jan 13, 2018

gavanderhoorn left a comment

Choose a reason for hiding this comment

inigomartinez commented Jan 15, 2018

gavanderhoorn commented Jan 15, 2018

nuclearsandwich commented Jan 16, 2018

nuclearsandwich commented Jan 12, 2018 •

edited

Loading

nuclearsandwich commented Jan 13, 2018 •

edited

Loading