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

Manually building and installing docker-api gem #1122

Merged
merged 1 commit into from
Apr 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions components/gems/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,11 @@ group(:omnibus_package) do
gem "kitchen-digitalocean", ">= 0.10.0"
gem "kitchen-dokken", ">= 2.8.1"
# docker-api appears to be unmaintained, and has a noisy deprecation
# warning under ruby 2.7. Use a local fork. Dependency of kitchen-dokken.
gem "docker-api", git: "https://github.com/chef/docker-api.git"
# warning under ruby 2.7. Using a fork via an omnibus software def.
# Must use the software def because the ruby cleanup deletes the
# bundled gem. Chef Infra Client does not have this issue with Ohai
# and I do not understand why.
Copy link
Contributor

Choose a reason for hiding this comment

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

The client uses the ohai software definition to install ohai from git.

If you're installing from git it gets installed into the bundler gitsha'd directory and not into the proper location where normal rubygems can see it. Bundler sees it is from a git source and therefore untrusted so refuses to pollute your prestine set of gems from rubygems with that prerelease gem. That later gets cleaned up by the omnibus ruby cleanup scripts, but it never would have worked anyway.

So this is the right thing to do for git sources.

IMO this is screaming out that we need to either:

  1. fork it to chef-docker-api gem, fully take it over, and release it to rubygems.

  2. stop using the ruby API entirely and shell out to the docker command line.

I am mostly in favor of #2 (up until it is shown that the docker command line API is as big of a tirefire as dnf/yum or ruby is anyway). Until its shown that the CLI there is painfully slow, the benefits of a ruby library solution over shelling out to the command line are mostly philosophical rather than practical. The practical matter is that it ties you to that API gem being updated properly which is the net-ssh problem all over again.

Copy link
Contributor

Choose a reason for hiding this comment

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

We also have to do this, but as long as you're not pushing anything to the docker-api gem the version in the Gemfile.lock that appbundler reads and the master version here shouldn't differ so you shouldn't need it. But if you're actively developing in the docker-api gem you'll need this as well:

https://github.com/chef/chef/blob/master/omnibus_overrides.rb#L27-L33

Copy link
Contributor

@lamont-granquist lamont-granquist Apr 24, 2020

Choose a reason for hiding this comment

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

Again, this is due to bundler and that stupid directory it installs git gems into. The gem build; gem install workaround is correct since that sends the gem into the right lib location. The problem is that may also pull all kinds of duplicated gems into the shipping artifact depending on the pins in the Gemfile.lock -- if you have any dependent gems in docker-api which are held back by other pins (e.g. inspec) that is how you get duplicated gems.

And I think that will cause Tim at least to favor one of the other two approaches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up having to remove the git sourcing from the Gemfile because that caused appbundling to fail with the error

---- Begin output of bundle lock ----
--
  | STDOUT: Fetching gem metadata from http://k8s-artifactory-production.es.chef.co/artifactory/api/gems/rubygems-proxy/........
  | Fetching gem metadata from http://k8s-artifactory-production.es.chef.co/artifactory/api/gems/rubygems-proxy/..
  | Could not find gem 'docker-api' in source at
  | `/opt/chef-workstation/embedded/lib/ruby/gems/2.6.0/gems/docker-api-1.34.2`.
  | The source does not contain any versions of 'docker-api'

I don't know how that was failing because I added a debug line to verify that the gem was being correctly installed from the omnibus software definition:

[Builder: docker-api] I | 2020-04-23T22:56:40+00:00 | $ /opt/chef-workstation/embedded/bin/gem install docker-api-*.gem --no-document --ignore-dependencies
                      D | 2020-04-23T22:56:41+00:00 | Successfully installed docker-api-1.34.2
                      D | 2020-04-23T22:56:41+00:00 | 1 gem installed
[Builder: docker-api] I | 2020-04-23T22:56:41+00:00 | gem `install docker-api-*.gem --no-document --ignore-dependencies': 0.2902s
[Builder: docker-api] I | 2020-04-23T22:56:41+00:00 | $ ls -al /opt/chef-workstation/embedded/lib/ruby/gems/2.6.0/gems/
                      D | 2020-04-23T22:56:41+00:00 | total 56
                      D | 2020-04-23T22:56:41+00:00 | drwxr-xr-x 14 buildkite-agent buildkite-agent 4096 Apr 23 22:56 .
                      D | 2020-04-23T22:56:41+00:00 | drwxr-xr-x  8 buildkite-agent buildkite-agent 4096 Apr 23 22:53 ..
                      D | 2020-04-23T22:56:41+00:00 | drwxr-xr-x  5 buildkite-agent buildkite-agent 4096 Apr 23 22:53 bundler-1.17.2
                      D | 2020-04-23T22:56:41+00:00 | drwxr-xr-x  7 buildkite-agent buildkite-agent 4096 Apr 23 22:53 did_you_mean-1.3.0
                      D | 2020-04-23T22:56:41+00:00 | drwxr-xr-x  3 buildkite-agent buildkite-agent 4096 Apr 23 22:56 docker-api-1.34.2

gem "docker-api" # , git: "https://github.com/chef/docker-api.git", branch: "master"
gem "kitchen-google", ">= 2.0.0"
gem "kitchen-hyperv", ">= 0.5.1"
gem "kitchen-inspec", ">= 1.0"
Expand Down
13 changes: 4 additions & 9 deletions components/gems/Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
GIT
remote: https://github.com/chef/docker-api.git
revision: afa9cc675cee491ef70315600de923fcb07b3ea3
specs:
docker-api (1.34.2)
excon (>= 0.47.0)
multi_json

GEM
remote: https://rubygems.org/
specs:
Expand Down Expand Up @@ -369,6 +361,9 @@ GEM
descendants_tracker (0.0.4)
thread_safe (~> 0.3, >= 0.3.1)
diff-lcs (1.3)
docker-api (1.34.2)
excon (>= 0.47.0)
multi_json
domain_name (0.5.20190701)
unf (>= 0.0.5, < 1.0.0)
droplet_kit (3.7.0)
Expand Down Expand Up @@ -1049,7 +1044,7 @@ DEPENDENCIES
cookstyle (~> 5.20)
dep-selector-libgecode
dep_selector
docker-api!
docker-api
ed25519
fauxhai-ng (~> 7.5)
ffi-libarchive
Expand Down
38 changes: 38 additions & 0 deletions omnibus/config/software/docker-api.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#
# Copyright 2012-2020 Chef Software, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

name "docker-api"
default_version "master"

source git: "https://github.com/chef/docker-api.git"

license "MIT"
license_file "https://github.com/raw/chef/docker-api/master/LICENSE"

dependency "ruby"

build do
env = with_standard_compiler_flags(with_embedded_path)

# docker-api does not have any unique dependencies not already
# included in Chef Workstation. We let the master gem bundle
# install those gems. If we ever add custom dependencies into
# our fork we need to re-add bundle install here for make the
# gemfile pull from the fork and don't build here.
# bundle "install", env: env
gem "build docker-api.gemspec", env: env
gem "install docker-api-*.gem --no-document --ignore-dependencies", env: env
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lamont-granquist I know that doing a gem build; gem install from an omnibus definition can end up installing duplicate gems, so I added --ignore-dependencies here. Its not a good long-term solution, but I'm out of time to troubleshoot this. We currently only have 1 difference from master in our fork but if we start making other changes to that gem we will need to re-address how we do this.

end
6 changes: 6 additions & 0 deletions omnibus/config/software/gems.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@
# @todo Contact gem mainter about getting new release.
dependency "rb-fsevent-gem" if mac_os_x?

# The docker-api gem has not been maintained so we are building and installing
# a fork of it. If the maintainer releases a fix with our change we can switch
# back to just installing it. The docker-api gem is required by kitchen-dokken
# in its gemspec.
dependency "docker-api"

build do
env = with_standard_compiler_flags(with_embedded_path)

Expand Down