Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't install erlang-ftp and erlang-tftp to fix xenial #532

Merged
merged 3 commits into from
May 22, 2019
Merged

Don't install erlang-ftp and erlang-tftp to fix xenial #532

merged 3 commits into from
May 22, 2019

Conversation

stefansundin
Copy link
Contributor

Proposed Changes

Hi, this is as much a question as a possible solution. It does not appear that erlang-ftp and erlang-tftp are available for Ubuntu 16.04 (xenial) in the rabbitmq-erlang repository.

The files erlang-ftp_21.3.8.2-1_amd64.deb and erlang-tftp_21.3.8.2-1_amd64.deb are available for bionic, but not for xenial.

I don't know what these packages are used for in RabbitMQ, but from what I can gather, things appear to be working without them.

Types of Changes

  • Bug fix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (correction or otherwise)
  • Cosmetics (whitespace, appearance)

Checklist

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

Further Comments

Please tell me if I'm doing something wrong. :)

Fixes this error:

  * rabbitmq_erlang_package_from_bintray[rabbitmq_erlang] action install
    * apt_package[erlang-base, erlang-mnesia, erlang-runtime-tools, erlang-asn1, erlang-crypto, erlang-public-key, erlang-ssl, erlang-syntax-tools, erlang-snmp, erlang-os-mon, erlang-parsetools, erlang-ftp, erlang-tftp, erlang-inets, erlang-tools, erlang-eldap, erlang-xmerl, erlang-dev, erlang-edoc, erlang-eunit, erlang-erl-docgen, erlang-src] action install
      * No candidate version available for erlang-ftp, erlang-tftp
      ================================================================================
      Error executing action `install` on resource 'apt_package[erlang-base, erlang-mnesia, erlang-runtime-tools, erlang-asn1, erlang-crypto, erlang-public-key, erlang-ssl, erlang-syntax-tools, erlang-snmp, erlang-os-mon, erlang-parsetools, erlang-ftp, erlang-tftp, erlang-inets, erlang-tools, erlang-eldap, erlang-xmerl, erlang-dev, erlang-edoc, erlang-eunit, erlang-erl-docgen, erlang-src]'
      ================================================================================

      Chef::Exceptions::Package
      -------------------------
      No candidate version available for erlang-ftp, erlang-tftp

      Resource Declaration:
      ---------------------
      # In /etc/chef/local-mode-cache/cache/cookbooks/rabbitmq/providers/erlang_package_from_bintray.rb

       52:     apt_package(erlang_packages) do
       53:       options new_resource.options unless new_resource.options.nil?
       54:       # must provide an array of versions!
       55:       version erlang_packages.map { new_resource.version } unless new_resource.version.nil?
       56:       retries new_resource.retries
       57:       retry_delay new_resource.retry_delay unless new_resource.retry_delay.nil?
       58:       action :install
       59:     end
       60:   end

…u 16.04 (xenial) in the rabbitmq-erlang repository.
Copy link
Member

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for digging in.

A bit of research suggests that the packages was introduced (extracted from inets?) in Erlang 21. Since we base our packages on the original Debian ones, I suspect that the ones for Xenial exclude the package even when we build Erlang 21+ (which Xenial never packaged but we do).

Can exclude those packages from the install list for

  • Erlang versions < 21
  • Xenial and its respective Debian counterpart

instead? Or use it as a base and add those packages for distributions newer than Xenial/its Debian counterpart?

We'd need to run Kitchen tests on a lot more distributions it seems :(

…me, since they are not available for xenial.
@stefansundin
Copy link
Contributor Author

I pushed a change to remove them from the list if lsb_codename == xenial. It appears that buster, stretch, and bionic all have erlang-ftp and erlang-tftp.

Also, feel free to modify my branch if needed.

@michaelklishin michaelklishin merged commit 8c6205b into rabbitmq:v5.x May 22, 2019
@michaelklishin
Copy link
Member

All Erlang package tests pass (including on Xenial). Thank you!

@michaelklishin michaelklishin added this to the 5.8.2 milestone May 22, 2019
@stefansundin
Copy link
Contributor Author

Hmm.. Do we need to add the same thing to the action :remove section perhaps? I have to admit that I don't know how to trigger that code since I have only been installing it so far by including the recipes. Maybe it's ok to try to remove a package that isn't installed?

By the way, what are these packages even used for? Does rabbitmq ever use any ftp functionality?

@michaelklishin
Copy link
Member

Yes, we should do the same thing for :remove. To trigger that code you have to do two Chef runs, one that installs the package and another one that removes it. In fact, two resources one after another might do it in a single run.

The package is not used by RabbitMQ but it is a dependency of erlang-inets which includes, among other things, an HTTP client. So we cannot drop erlang-inets.

michaelklishin added a commit that referenced this pull request May 25, 2019
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.

2 participants