Drop dynflow_core support #720

evgeni · 2022-01-05T18:50:52Z

it's gone since smart_proxy_dynflow 0.4.0

evgeni · 2022-01-05T18:51:40Z

manifests/plugin/dynflow.pp

-    file { '/etc/smart_proxy_dynflow_core/settings.yml':
-      ensure  => file,
-      content => template('foreman_proxy/plugin/dynflow_core.yml.erb'),
-      require => Foreman_proxy::Plugin['dynflow_core'],
-      notify  => Service[$service],
-    }
-
-    file { '/etc/smart_proxy_dynflow_core/settings.d':
-      ensure  => link,
-      target  => "${foreman_proxy::config_dir}/settings.d",
-      require => Foreman_proxy::Plugin['dynflow_core'],
-      notify  => Service[$service],
-    }


instead of removing the code, we could also ensure → absent for a cleanup, what do you think?

Cleanup sounds reasonable to me. Otoh would we still want to eventually drop it?

Is this normally owned by a package? If so, can we ensure the package is absent instead?

settings.yml was owned by the package (see https://github.com/theforeman/foreman-packaging/pull/6753/files), but settings.d not, I think.

On the other hand, it's probably marked as a config file so it would stay as .rpmsave so cleaning it up is likely needed.

Hah, I thought about that at lunch, but then forgot again when I started typing the commit update…

evgeni · 2022-01-05T18:51:49Z

manifests/plugin/dynflow.pp

-  service { 'smart_proxy_dynflow_core':
-    ensure => $external_core,
-    enable => $external_core,
-  }


instead of removing the code, we could also ensure → absent for a cleanup, what do you think?

The service is owned by the package that should be removed, right? Removing the package is the best way to get rid of the service.

At least in the last version of the package I have here:

# rpm -ql rubygem-smart_proxy_dynflow_core /usr/share/gems/gems/smart_proxy_dynflow_core-0.4.1 /usr/share/gems/gems/smart_proxy_dynflow_core-0.4.1/Gemfile.in /usr/share/gems/gems/smart_proxy_dynflow_core-0.4.1/LICENSE /usr/share/gems/gems/smart_proxy_dynflow_core-0.4.1/lib /usr/share/gems/gems/smart_proxy_dynflow_core-0.4.1/lib/smart_proxy_dynflow_core /usr/share/gems/gems/smart_proxy_dynflow_core-0.4.1/lib/smart_proxy_dynflow_core.rb /usr/share/gems/gems/smart_proxy_dynflow_core-0.4.1/lib/smart_proxy_dynflow_core/task_launcher_registry.rb /usr/share/gems/gems/smart_proxy_dynflow_core-0.4.1/lib/smart_proxy_dynflow_core/version.rb /usr/share/gems/gems/smart_proxy_dynflow_core-0.4.1/smart_proxy_dynflow_core.gemspec /usr/share/gems/specifications/smart_proxy_dynflow_core-0.4.1.gemspec

0.4.1 was already an compatibility-only shim mapping constants to dfifferent names. From a version we shipped in 2.4:

# rpm -ql ./tfm-rubygem-smart_proxy_dynflow_core-0.3.3-1.fm2_4.el7.noarch.rpm /etc/logrotate.d/tfm-rubygem-smart_proxy_dynflow_core /etc/smart_proxy_dynflow_core/settings.yml /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3 /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/Gemfile.in /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/LICENSE /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/bin /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/bin/smart_proxy_dynflow_core /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/config /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/config/settings.yml.example /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/api.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/bundler_helper.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/callback.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/core.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/helpers.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/launcher.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/log.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/logger_middleware.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/middleware /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/middleware/keep_current_request_id.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/request_id_middleware.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/settings.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/task_launcher_registry.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/testing.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/lib/smart_proxy_dynflow_core/version.rb /opt/theforeman/tfm/root/usr/share/gems/gems/smart_proxy_dynflow_core-0.3.3/smart_proxy_dynflow_core.gemspec /opt/theforeman/tfm/root/usr/share/gems/specifications/smart_proxy_dynflow_core-0.3.3.gemspec /opt/theforeman/tfm/root/usr/share/smart_proxy_dynflow_core/bundler.d /usr/bin/smart_proxy_dynflow_core /usr/lib/systemd/system/smart_proxy_dynflow_core.service

but it did own that file before, so it's probably cleaned up on removal/upgrade, you're right

https://github.com/theforeman/foreman-packaging/pull/6753/files

evgeni · 2022-01-05T18:55:02Z

@adamruzicka @ekohl seems we forgot something while dropping _core ;)

evgeni · 2022-01-05T19:44:23Z

this is as green as it gets without me getting really dirty with psych.

ekohl · 2022-01-06T10:43:10Z

templates/plugin/dynflow.yml.erb

+# Require a valid cert to access Dynflow console
+:console_auth: <%= scope.lookupvar('::foreman_proxy::plugin::dynflow::console_auth') %>


Is this a regression that you're fixing?

no, it was in core yml before.

or do you mean in a non-external-core scenario? heck, I don't know? ;)

I was following the lines of theforeman/smart_proxy_dynflow@e205e19#diff-8a58542e75188035fcfefaf9049586f5665b71ae0dabe5f533a61d3719f00e78

or do you mean in a non-external-core scenario? heck, I don't know? ;)

Indeed. It looks like that wasn't implemented. However, it defaults to true in the plugin itself and is only used to disable authentication. Not really important to cherry pick then.

evgeni · 2022-01-06T11:09:09Z

clean up should be as simple as

file { '/etc/smart_proxy_dynflow_core/settings.d':
  ensure => absent
} ->
foreman_proxy::plugin { 'dynflow_core':
  version  => 'absent',
}

right?

ekohl · 2022-01-06T12:19:41Z

clean up should be as simple as

That looks good. The chaining is indeed a good idea because it probably means that /etc/smart_proxy_dynflow_core ends up being removed as well.

evgeni · 2022-01-06T12:22:04Z

clean up should be as simple as

That looks good. The chaining is indeed a good idea because it probably means that /etc/smart_proxy_dynflow_core ends up being removed as well.

That was indeed the intention

it's gone since smart_proxy_dynflow 0.4.0

ekohl

I think you need to rebase on master since I just merged the pin-rdoc branch. Otherwise 👍

evgeni · 2022-01-06T13:43:38Z

I think you need to rebase on master since I just merged the pin-rdoc branch. Otherwise +1

git is smart enough, and you thankfully used the exact diff I had in my commit ;)

theforeman-bot added Needs testing Not yet reviewed labels Jan 5, 2022

evgeni commented Jan 5, 2022

View reviewed changes

evgeni force-pushed the drop-dynflow-core branch from 160f3b8 to 1b9e75b Compare January 5, 2022 18:53

evgeni force-pushed the drop-dynflow-core branch from 13f27a8 to 2e3c7bb Compare January 5, 2022 19:37

evgeni added the Backwards incompatible label Jan 5, 2022

ekohl reviewed Jan 6, 2022

View reviewed changes

evgeni mentioned this pull request Jan 6, 2022

Pin rdoc to < 6.4 theforeman/foreman-installer-modulesync#142

Merged

13 tasks

evgeni force-pushed the drop-dynflow-core branch 3 times, most recently from 8933f88 to 32055ea Compare January 6, 2022 13:18

evgeni added 2 commits January 6, 2022 14:23

drop dynflow_core support

91a3a97

it's gone since smart_proxy_dynflow 0.4.0

pin rdoc to prevent pulling in psych that breaks the world

03495d6

evgeni force-pushed the drop-dynflow-core branch from 32055ea to 03495d6 Compare January 6, 2022 13:23

ekohl approved these changes Jan 6, 2022

View reviewed changes

theforeman-bot removed the Not yet reviewed label Jan 6, 2022

evgeni merged commit 7c27c94 into theforeman:master Jan 6, 2022

evgeni deleted the drop-dynflow-core branch January 6, 2022 13:43

ekohl changed the title ~~drop dynflow_core support~~ Drop dynflow_core support Feb 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop dynflow_core support #720

Drop dynflow_core support #720

evgeni commented Jan 5, 2022

evgeni Jan 5, 2022

adamruzicka Jan 6, 2022

ekohl Jan 6, 2022

evgeni Jan 6, 2022

ekohl Jan 6, 2022

evgeni Jan 6, 2022

evgeni Jan 5, 2022

ekohl Jan 6, 2022

evgeni Jan 6, 2022

adamruzicka Jan 6, 2022

evgeni Jan 6, 2022

evgeni commented Jan 5, 2022

evgeni commented Jan 5, 2022

ekohl Jan 6, 2022

evgeni Jan 6, 2022

ekohl Jan 6, 2022

evgeni commented Jan 6, 2022 •

edited

Loading

ekohl commented Jan 6, 2022

evgeni commented Jan 6, 2022

ekohl left a comment

evgeni commented Jan 6, 2022

		# Require a valid cert to access Dynflow console
		:console_auth: <%= scope.lookupvar('::foreman_proxy::plugin::dynflow::console_auth') %>

Drop dynflow_core support #720

Drop dynflow_core support #720

Conversation

evgeni commented Jan 5, 2022

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

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

Choose a reason for hiding this comment

evgeni commented Jan 5, 2022

evgeni commented Jan 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgeni commented Jan 6, 2022 • edited Loading

ekohl commented Jan 6, 2022

evgeni commented Jan 6, 2022

ekohl left a comment

Choose a reason for hiding this comment

evgeni commented Jan 6, 2022

evgeni commented Jan 6, 2022 •

edited

Loading