Skip to content

Conversation

@ehelms
Copy link
Member

@ehelms ehelms commented Jul 9, 2021

@adamruzicka @evgeni @ekohl Please take a look

The interesting part here is, and I don't know the code enough, is that by smart_proxy_remote_execution_ssh obsoleting foreman_remote_execution_core we are introducing functionality that was not present before. That is, previously only the core code parts existed via the foreman_remote_execution_core gem. Now we are introducing the actual smart_proxy_remote_execution plugin even though a user may not have had it configured.

On upgrade, the yum output is:

 tfm-rubygem-smart_proxy_remote_execution_ssh                                      noarch                                      0.4.1-1.fm2_6.el7                                                                          foreman-plugins-koji                                       35 k
     replacing  tfm-rubygem-foreman_remote_execution_core.noarch 1.4.3-1.el7

This is, what in turn results in:

# cat /etc/foreman-proxy/settings.d/remote_execution_ssh.yml 
---
:enabled: true
:ssh_identity_key_file: '~/.ssh/id_rsa_foreman_proxy'
:local_working_dir: '/var/tmp'
:remote_working_dir: '/var/tmp'
# :kerberos_auth: false
# :async_ssh: false

# Defines how often (in seconds) should the runner check
# for new data leave empty to use the runner's default
# (1 second for regular, 60 seconds with async_ssh enabled)
# :runner_refresh_interval:

# Defines the verbosity of logging coming from Net::SSH
# one of :debug, :info, :warn, :error, :fatal
# must be lower than general log level
# :ssh_log_level: fatal

# Remove working directories on job completion
# :cleanup_working_dirs: true

Even though, a user has:

# grep -r remote_execution::ssh /etc/foreman-installer/scenarios.d/katello-answers.yaml
foreman_proxy::plugin::remote_execution::ssh: false

So I think that this is what was breaking the upgrade before:
https://github.com/theforeman/foreman-packaging/pull/6855/files#diff-5639bb35f3b9b6c5d8eb5121c28b627340d9fa15bbee42e94682fc58cb51a4d6L24

And now, the Obsoletes is breaking us. Something maybe ought to clean up the system, but not the smart proxy plugin.

@ekohl
Copy link
Member

ekohl commented Jul 9, 2021

I also submitted theforeman/smart_proxy_remote_execution_ssh#59 which means the RPM can be installed (as a library) but not enabled.

@ehelms
Copy link
Member Author

ehelms commented Jul 9, 2021

Thats the part I am fuzzy on... is smart_proxy_remote_execution_ssh used as a library? If it is, I agree it should be disabled by default to confer the wrong intent. If its a legit stand-alone smart-proxy plugin that provides a feature then it shouldn't get auto-installed on the user system if they did not have it prior and we should let the installer handle enable / disable.

@adamruzicka
Copy link

by smart_proxy_remote_execution_ssh obsoleting foreman_remote_execution_core we are introducing functionality that was not present before

Not exactly. There was a time when remote execution could run from foreman itself or through a smart proxy. The code for this lived in foreman_remote_execution_core. Because of this foreman_remote_execution_core was a dependency of "regular" foreman_remote_execution. We later dropped this model and enforced that execution always has to go through a smart proxy, but noone removed the dependency so if you had foreman_remote_execution, you also had foreman_remote_execution_core.

Fast forward to ~two weeks ago, we moved contents of foreman_remote_execution_core into smart_proxy_remote_execution_ssh, but kept foreman_remote_execution_core around as a compatibility layer, exposing the moved code from smart_proxy_remote_execution_ssh under different names for plugins which still depended on the old foreman_remote_execution_core. Because of this, foreman_remote_execution_core had to depend on smart_proxy_remote_execution_ssh. So if you had REX in foreman, but not on the proxy, this is what started bringing in functionality that was no present before.

is smart_proxy_remote_execution_ssh used as a library?

No, it is a legit plugin that provides a feature.

Now that no other proxy plugin (barring acd) depends on foreman_remote_execution_core how about we:

  • keep obsoleting foreman_remote_execution_core from new-enough smart_proxy_remote_execution_ssh
  • yank the version of foreman_remote_execution_core which required smart_proxy_remote_execution_ssh from nightly repos

As far as I understand it this is a by-product of the upgrade from "rex core" through "rex core as compatibility layer to smart_proxy_remote_execution_ssh" to "only smart_proxy_remote_execution_ssh". If we skip the middle step, things should sort themselves out.

Unless I'm missing something this should allow us to get out of this mess. If folks do a 2.5->2.6 upgrade and have rex on the proxy, foreman_remote_execution_core will be removed and things should continue working. If they don't have rex on the proxy, the old foreman_remote_execution_core package will stick around but will not actually be required by anything.

I know it sounds like a hack, but I'd prefer a one time hack over introducing non-standard behavior (proxy plugin not being enabled by default) for the long term. Opinions?

@evgeni
Copy link
Member

evgeni commented Jul 12, 2021

  • keep obsoleting foreman_remote_execution_core from new-enough smart_proxy_remote_execution_ssh
  • yank the version of foreman_remote_execution_core which required smart_proxy_remote_execution_ssh from nightly repos

This won't help. 😿

The problem (as I see it, is the following):

Which forms the following questions:

  • Are there legit scenarios, where users had REX installed, but no proxy-rex-ssh?
    • Katello without using REX (but installed via dependency)
    • Katello with rex-ssh on external proxy
    • yes
  • Do these users need rex-ssh on the internal proxy now?
    • I don't see a need. If they didn't use it before, they won't need it now.
  • Should we listen to Eric and drop the "Obsoletes", thus not adding features to the setup that weren't used before?
    • Probably yes.

@adamruzicka
Copy link

REX depends on REX core (also in Foreman 2.5):

But not in nightly/> 2.5, no?

Now we either make REX core depend on proxy-rex-ssh, or proxy-rex-ssh Obsoletes REX core, but in both cases yum detects: oh, now I need to install proxy-rex-ssh (if it's not already there)

If we go with obsolete, why would yum want to install proxy-rex-ssh if it is not already there? What would pull it in?

@evgeni
Copy link
Member

evgeni commented Jul 12, 2021

REX depends on REX core (also in Foreman 2.5):

But not in nightly/> 2.5, no?

No, but our problem are upgraded setups from 2.5.
So the system has rex and rex core installed, regardless of what nightly defines.

Now we either make REX core depend on proxy-rex-ssh, or proxy-rex-ssh Obsoletes REX core, but in both cases yum detects: oh, now I need to install proxy-rex-ssh (if it's not already there)

If we go with obsolete, why would yum want to install proxy-rex-ssh if it is not already there? What would pull it in?

If you say "package B obsoletes package A" yum translates this to: "if package A is installed, uninstall package A, install package B"

@adamruzicka
Copy link

If you say "package B obsoletes package A" yum translates this to: "if package A is installed, uninstall package A, install package B"

Oh. Oh... I thought it means "A and B can't live on the same machine, if they are, B wins". That explains a lot.

@evgeni
Copy link
Member

evgeni commented Jul 12, 2021

If you say "package B obsoletes package A" yum translates this to: "if package A is installed, uninstall package A, install package B"

Oh. Oh... I thought it means "A and B can't live on the same machine, if they are, B wins". That explains a lot.

That would be Conflicts, with the caveat that yum can't resolve conflicts automatically ("A is installed, B conflicts A, user wants B, automatically uninstall A") and will just puke in your face if you ask it to. (And then, it's not really "B wins", it's really just "A and B can't co-exist")

@adamruzicka
Copy link

Then agreed. Should we also drop the smart_proxy_ansible-foreman_ansible_core obsolete?

@evgeni
Copy link
Member

evgeni commented Jul 12, 2021

Then agreed. Should we also drop the smart_proxy_ansible-foreman_ansible_core obsolete?

Yeah, I think the same should happen there, even if there is less of a source for problems there.

@evgeni evgeni merged commit b233383 into theforeman:rpm/develop Jul 12, 2021
@ekohl
Copy link
Member

ekohl commented Jul 12, 2021

In SCL we have a tfm package which can obsolete it. That's guaranteed to be installed. However, we don't have a similar thing for EL8.

@evgeni
Copy link
Member

evgeni commented Jul 12, 2021

In SCL we have a tfm package which can obsolete it. That's guaranteed to be installed. However, we don't have a similar thing for EL8.

Yeah, and in this case it warrants a removal on EL8 too

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants