Skip to content

[master] rpm: fix circular dependency between scan-cli-plugin and docker-ce-cli#657

Merged
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:fix_scan_dependency
Mar 27, 2022
Merged

[master] rpm: fix circular dependency between scan-cli-plugin and docker-ce-cli#657
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:fix_scan_dependency

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

This fixes a problem when installing the packages from local files instead of
from the package repository. The current packages had a strong dependency set
in both directions (docker-ce-cli requires docker-scan-plugin, and vice-
versa). This caused a circular dependency, which could not be resolved when
trying to install the packages offline (from downloaded rpm files);

yum install ./docker-scan-plugin-0.17.0-3.fc35.x86_64.rpm
Last metadata expiration check: 1:09:40 ago on Tue Mar  8 08:30:47 2022.
Error:
Problem: conflicting requests
- nothing provides docker-ce-cli needed by docker-scan-plugin-0.17.0-3.fc35.x86_64
  (try to add '--skip-broken' to skip uninstallable packages)

yum install ./docker-ce-cli-20.10.13-3.fc35.x86_64.rpm
Last metadata expiration check: 1:10:23 ago on Tue Mar  8 08:30:47 2022.
Error:
Problem: conflicting requests
- nothing provides docker-scan-plugin(x86-64) needed by docker-ce-cli-1:20.10.13-3.fc35.x86_64
  (try to add '--skip-broken' to skip uninstallable packages)

This patch

  • changes the docker-scan-plugin to `Enhances
  • changes the docker-ce-cli package to mark the scan plugin as Recommends,
    with the exception of CentOS/RHEL 7, which do not yet support weak dependencies.
    For those, we continue to use Requires.

The Recommends dependency should be installed by default, but users can opt-out
by passing --setopt=install_weak_deps=False to dnf.

Packager: Docker <[email protected]>

Requires: docker-ce-cli
Enhances: docker-ce-cli

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Only thinking if this should be set conditional as well (not sure if older versions just ignore it, or if they would warn/complain), i.e.;

Suggested change
Enhances: docker-ce-cli
# CentOS 7 and RHEL 7 do not yet support weak dependencies
%if 0%{?rhel} != 7
Enhances: docker-ce-cli
%endif

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok; looks like this is needed

error: line 15: Unknown tag: Enhances: docker-ce-cli

# TODO change once we support scan-plugin on other architectures
%ifarch x86_64
# CentOS 7 and RHEL 7 do not yet support weak dependencies
%if 0%{?rhel} == 7

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

note that I'm specifically not using < or > here, as rhel won't be / may not be defined on other RPM platforms, in which case 0 (default) is picked

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

^^ added a note about that in the comment to prevent someone accidentally changing it

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I only hope this will always be 7 and not 7.1 (e.g.) on CentOS / RHEL 🤔

# TODO change once we support scan-plugin on other architectures
%ifarch x86_64
# CentOS 7 and RHEL 7 do not yet support weak dependencies
%if 0%{?rhel} == 7

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
%if 0%{?rhel} == 7
%if ! (0%{?rhel} && 0%{?rhel} < 7)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually your cond looks right.

This fixes a problem when installing the packages from local files instead of
from the package repository. The current packages had a strong dependency set
in both directions (`docker-ce-cli` requires `docker-scan-plugin`, and vice-
versa). This caused a circular dependency, which could not be resolved when
trying to install the packages offline (from downloaded rpm files);

    yum install ./docker-scan-plugin-0.17.0-3.fc35.x86_64.rpm
    Last metadata expiration check: 1:09:40 ago on Tue Mar  8 08:30:47 2022.
    Error:
    Problem: conflicting requests
    - nothing provides docker-ce-cli needed by docker-scan-plugin-0.17.0-3.fc35.x86_64
      (try to add '--skip-broken' to skip uninstallable packages)

    yum install ./docker-ce-cli-20.10.13-3.fc35.x86_64.rpm
    Last metadata expiration check: 1:10:23 ago on Tue Mar  8 08:30:47 2022.
    Error:
    Problem: conflicting requests
    - nothing provides docker-scan-plugin(x86-64) needed by docker-ce-cli-1:20.10.13-3.fc35.x86_64
      (try to add '--skip-broken' to skip uninstallable packages)

This patch

- changes the `docker-scan-plugin` to `Enhances
- changes the `docker-ce-cli` package to mark the scan plugin as `Recommends`,
  with the exception of CentOS/RHEL 7, which do not yet support weak dependencies.
  For those, we continue to use `Requires`.

The `Recommends` dependency should be installed by default, but users can opt-out
by passing `--setopt=install_weak_deps=False` to `dnf`.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
thaJeztah force-pushed the fix_scan_dependency branch from 70fd056 to 51098f9 Compare March 27, 2022 11:36

@crazy-max crazy-max left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Resolving Dependencies
--> Running transaction check
---> Package docker-ce-cli.x86_64 1:22.04.0-3.el7 will be installed
--> Processing Dependency: docker-scan-plugin(x86-64) for package: 1:docker-ce-cli-22.04.0-3.el7.x86_64
--> Running transaction check
---> Package docker-scan-plugin.x86_64 0:0.17.0-3.el7 will be installed
--> Finished Dependency Resolution

Dependencies Resolved

========================================================================================================================================================================================================== Package                                               Arch                                      Version                                             Repository                                      Size 
==========================================================================================================================================================================================================Installing:
 docker-ce-cli                                         x86_64                                    1:22.04.0-3.el7                                     docker-test                                     18 M 
Installing for dependencies:
 docker-scan-plugin                                    x86_64                                    0.17.0-3.el7                                        docker-test                                    3.7 M 

Transaction Summary
==========================================================================================================================================================================================================Install  1 Package (+1 Dependent package)

Total download size: 22 M
Installed size: 96 M
Is this ok [y/d/N]: y
Downloading packages:
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------Total                                                                                                                                                                     410 MB/s |  22 MB  00:00:00     
Running transaction check
Running transaction test
Transaction test succeeded
Running transaction
  Installing : docker-scan-plugin-0.17.0-3.el7.x86_64                                                                                                                                                 1/2 
  Installing : 1:docker-ce-cli-22.04.0-3.el7.x86_64                                                                                                                                                   2/2 
  Verifying  : 1:docker-ce-cli-22.04.0-3.el7.x86_64                                                                                                                                                   1/2 
  Verifying  : docker-scan-plugin-0.17.0-3.el7.x86_64                                                                                                                                                 2/2 

Installed:
  docker-ce-cli.x86_64 1:22.04.0-3.el7

Dependency Installed:
  docker-scan-plugin.x86_64 0:0.17.0-3.el7

Complete!

@thaJeztah

Copy link
Copy Markdown
Member Author

Whoop! Let's get this one in

@thaJeztah
thaJeztah merged commit c6374f7 into docker:master Mar 27, 2022
@thaJeztah
thaJeztah deleted the fix_scan_dependency branch March 27, 2022 12:06
@thaJeztah

Copy link
Copy Markdown
Member Author

We should probably cherry-pick this one as well

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants