Fixes #29737 - Put smart proxy plugin local state into /var/lib/foreman-proxy #5197

adamruzicka · 2020-05-12T07:50:55Z

No description provided.

…an-proxy

adamruzicka · 2020-05-12T08:09:28Z

[2020-05-12T08:05:14.673Z] TASK [repoclosure : Run repoclosure for el8] ***********************************
[2020-05-12T08:05:14.673Z] fatal: [rubygem-smart_proxy_ansible]: FAILED! => changed=true 
[2020-05-12T08:05:14.673Z]   cmd:
[2020-05-12T08:05:14.673Z]   - dnf
[2020-05-12T08:05:14.673Z]   - repoclosure
[2020-05-12T08:05:14.673Z]   - --config
[2020-05-12T08:05:14.673Z]   - /tmp/ansible.gAM6TUrepoclosure/yum.conf
[2020-05-12T08:05:14.673Z]   - --refresh
[2020-05-12T08:05:14.673Z]   - --newest
[2020-05-12T08:05:14.673Z]   - --check
[2020-05-12T08:05:14.673Z]   - downloaded_rpms
[2020-05-12T08:05:14.673Z]   - --repofrompath=downloaded_rpms,./downloaded_rpms/el8
[2020-05-12T08:05:14.673Z]   - --repo
[2020-05-12T08:05:14.673Z]   - el8-baseos
[2020-05-12T08:05:14.673Z]   - --repo
[2020-05-12T08:05:14.673Z]   - el8-appstream
[2020-05-12T08:05:14.673Z]   - --repo
[2020-05-12T08:05:14.673Z]   - el8-extras
[2020-05-12T08:05:14.673Z]   - --repo
[2020-05-12T08:05:14.673Z]   - el8-foreman-nightly
[2020-05-12T08:05:14.673Z]   - --repo
[2020-05-12T08:05:14.673Z]   - el8-foreman-plugins-nightly
[2020-05-12T08:05:14.673Z]   delta: '0:00:05.747767'
[2020-05-12T08:05:14.673Z]   end: '2020-05-12 08:05:14.378789'
[2020-05-12T08:05:14.673Z]   msg: non-zero return code
[2020-05-12T08:05:14.673Z]   rc: 1
[2020-05-12T08:05:14.673Z]   start: '2020-05-12 08:05:08.631022'
[2020-05-12T08:05:14.673Z]   stderr: 'Error: Repoclosure ended with unresolved dependencies.'
[2020-05-12T08:05:14.673Z]   stderr_lines: <omitted>
[2020-05-12T08:05:14.673Z]   stdout: |-
[2020-05-12T08:05:14.673Z]     Added downloaded_rpms repo from ./downloaded_rpms/el8
[2020-05-12T08:05:14.673Z]     package: rubygem-smart_proxy_ansible-3.0.1-4.fm2_1.el8.noarch from downloaded_rpms
[2020-05-12T08:05:14.673Z]       unresolved deps:
[2020-05-12T08:05:14.673Z]         ansible >= 2.2
[2020-05-12T08:05:14.673Z]   stdout_lines: <omitted>

for some reason it seems to ignore the ansible repo, even though it was fixed yesterday

ekohl

Looks like a valid bug, but we can improve it a bit more. It's part of https://github.com/theforeman/foreman-packaging/blob/rpm/develop/gem2rpm/smart_proxy_plugin.spec.erb so it's in every spec file for a smart proxy plugin.

ekohl · 2020-05-12T09:55:00Z

packages/plugins/rubygem-smart_proxy_ansible/rubygem-smart_proxy_ansible.spec

@@ -11,14 +11,14 @@

 %global foreman_proxy_min_version 1.25
 %global foreman_proxy_dir %{_root_datadir}/foreman-proxy
-%global foreman_proxy_statedir %{_root_localstatedir}/foreman-proxy
+%global foreman_proxy_statedir %{_root_localstatedir}/lib/foreman-proxy


Looks like f825335 introduced this so will need a cherry pick to 2.0. Prior to it, it was %{_localstatedir}/lib/foreman-proxy. I think what was meant, was %{_sharedstatedir} (see https://docs.fedoraproject.org/en-US/packaging-guidelines/RPMMacros/).

Suggested change

%global foreman_proxy_statedir %{_root_localstatedir}/lib/foreman-proxy

%global foreman_proxy_statedir %{_root_sharedstatedir}/foreman-proxy

Note that the definition of _root_localstatedir should also be changed a few lines above then.

ekohl

Would you mind changing it in the gem2rpm template too and packages/plugins/rubygem-smart_proxy_*/*.spec so we're consistent? Otherwise we might see the same bug pop up again at some point in the future.

adamruzicka · 2020-05-12T11:30:38Z

I don't mind. Should I also bump release versions in all the specs I touch?

ekohl · 2020-05-12T11:33:10Z

Yes, I think that would be good.

ekohl

I think CI will shout at you for missing changelogs.

ekohl · 2020-05-12T11:48:24Z

gem2rpm/smart_proxy_plugin.spec.erb

 %{!?_root_sysconfdir:%global _root_sysconfdir %{_sysconfdir}}

 %global gem_name <%= spec.name %>
 %global plugin_name <%= spec.name.sub(/\Asmart_proxy_/, '') %>

 %global foreman_proxy_min_version 1.24
 %global foreman_proxy_dir %{_root_datadir}/foreman-proxy
-%global foreman_proxy_statedir %{_root_localstatedir}/foreman-proxy
+%global foreman_proxy_statedir %{_root_sharedstatedir}/foreman-proxy
 %global foreman_proxy_bundlerd_dir %{foreman_proxy_dir}/bundler.d
 %global foreman_proxy_settingsd_dir %{_root_sysconfdir}/foreman-proxy/settings.d
 %global smart_proxy_dynflow_bundlerd_dir %{!?scl:/opt/theforeman/tfm/root}%{_datadir}/smart_proxy_dynflow_core/bundler.d


Unrelated, but @zjhuntin this looks like it might break on EL8

Why is that?

Doesn't this mean it'll be prefixed with /opt/theforeman/tfm/root on EL8?

Seems to be working as expected if you compare:

http://koji.katello.org/koji/rpminfo?rpmID=340390
http://koji.katello.org/koji/rpminfo?rpmID=340393

packages/plugins/rubygem-smart_proxy_salt_core/rubygem-smart_proxy_salt_core.spec

ehelms · 2020-05-12T12:55:55Z

@adamruzicka I missed a spot when updating the repoclosure, https://github.com/theforeman/foreman-packaging/blob/rpm/develop/package_manifest.yaml#L639 also needs added el8-configmanagement-ansible

ekohl · 2020-05-12T14:04:07Z

Looks like salt is failing:

*** ERROR: ambiguous python shebang in /usr/sbin/upload-salt-reports: #!/usr/bin/env python. Change it to python3 (or python2) explicitly.

Not sure how this ever built for EL8.

ehelms · 2020-05-13T12:10:09Z

@adamruzicka If you could update the salt plugin, we can get this in. The way we've been doing it is:

https://github.com/theforeman/foreman-packaging/blob/rpm/develop/packages/foreman/foreman/foreman.spec#L725-L728

ekohl · 2020-05-13T22:58:37Z

packages/plugins/rubygem-smart_proxy_salt/rubygem-smart_proxy_salt.spec

+%if 0%{?fedora} || 0%{?rhel} >= 8
+sed -i 's|#!/usr/bin/env python|#!/usr/bin/python3|g' sbin/upload-salt-reports
+%endif


I think this needs to happen in the build phase. You're now modifying the source files, but in %install it's already %{gem_instdir}/sbin/upload-salt-reports:

*** ERROR: ambiguous python shebang in /usr/share/gems/gems/smart_proxy_salt-3.1.1/sbin/upload-salt-reports: #!/usr/bin/env python. Change it to python3 (or python2) explicitly. *** ERROR: ambiguous python shebang in /usr/sbin/upload-salt-reports: #!/usr/bin/env python. Change it to python3 (or python2) explicitly.

Yeah, I noticed late at night it didn't have the desired effect. I'll have to spin up an environment for buidling packages to reduce rtt for this

You can also submit a PR with just salt to get quicker testing.

👍 If you want to just pull salt out of this PR and put it on its own we can get the bulk of the changes in and you can iterate faster.

and bump release versions

adamruzicka · 2020-05-15T08:35:53Z

Took smart_proxy_salt out to deal with it separately

theforeman-bot added Needs testing Not yet reviewed RPM labels May 12, 2020

adamruzicka force-pushed the local-statedir branch from d0e176c to 9d9c5dd Compare May 12, 2020 07:54

Fixes #29737 - Put smart proxy plugin local state into /var/lib/forem…

f1f8903

…an-proxy

adamruzicka force-pushed the local-statedir branch from 9d9c5dd to f1f8903 Compare May 12, 2020 07:57

ekohl requested changes May 12, 2020

View reviewed changes

theforeman-bot added Waiting on contributor and removed Not yet reviewed labels May 12, 2020

Fixes #29737 - s/localstatedir/sharedstatedir/g

a7162c6

theforeman-bot added Needs re-review and removed Waiting on contributor labels May 12, 2020

ekohl requested changes May 12, 2020

View reviewed changes

theforeman-bot added Waiting on contributor and removed Needs re-review labels May 12, 2020

Fixes #29737 - Update gem2rpm template

a156810

theforeman-bot added Needs re-review and removed Waiting on contributor labels May 12, 2020

ekohl reviewed May 12, 2020

View reviewed changes

adamruzicka force-pushed the local-statedir branch 2 times, most recently from 37af15e to 560a5d6 Compare May 12, 2020 11:55

ekohl reviewed May 13, 2020

View reviewed changes

Fixes #29737 - s/localstatedir/sharedstatedir/g for all plugins

4cbbfd8

and bump release versions

adamruzicka force-pushed the local-statedir branch from 113199e to 4cbbfd8 Compare May 15, 2020 08:35

ehelms approved these changes May 15, 2020

View reviewed changes

theforeman-bot removed the Needs re-review label May 15, 2020

ehelms merged commit fed6312 into theforeman:rpm/develop May 15, 2020

adamruzicka deleted the local-statedir branch May 15, 2020 13:06

evgeni mentioned this pull request May 18, 2020

Fixes #29737 - Put smart proxy plugin local state into /var/lib/foreman-proxy #5240

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #29737 - Put smart proxy plugin local state into /var/lib/foreman-proxy #5197

Fixes #29737 - Put smart proxy plugin local state into /var/lib/foreman-proxy #5197

adamruzicka commented May 12, 2020

adamruzicka commented May 12, 2020

ekohl left a comment

ekohl May 12, 2020

ekohl left a comment

adamruzicka commented May 12, 2020

ekohl commented May 12, 2020

ekohl left a comment

ekohl May 12, 2020

ehelms May 12, 2020

ekohl May 12, 2020

ehelms May 12, 2020

ehelms commented May 12, 2020

ekohl commented May 12, 2020

ehelms commented May 13, 2020

ekohl May 13, 2020

adamruzicka May 14, 2020

ekohl May 14, 2020

ehelms May 14, 2020

adamruzicka commented May 15, 2020

	%global foreman_proxy_statedir %{_root_localstatedir}/lib/foreman-proxy
	%global foreman_proxy_statedir %{_root_sharedstatedir}/foreman-proxy

Fixes #29737 - Put smart proxy plugin local state into /var/lib/foreman-proxy #5197

Fixes #29737 - Put smart proxy plugin local state into /var/lib/foreman-proxy #5197

Conversation

adamruzicka commented May 12, 2020

adamruzicka commented May 12, 2020

ekohl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekohl left a comment

Choose a reason for hiding this comment

adamruzicka commented May 12, 2020

ekohl commented May 12, 2020

ekohl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehelms commented May 12, 2020

ekohl commented May 12, 2020

ehelms commented May 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamruzicka commented May 15, 2020