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.

Sign up for GitHub

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

Jump to bottom

Arch Linux: do not install openssh twice #58

Merged
merged 1 commit into from
Feb 11, 2022
Merged

Arch Linux: do not install openssh twice #58

merged 1 commit into from
Feb 11, 2022

Conversation

bastelfreak
Copy link
Member

No description provided.

@bastelfreak bastelfreak added the bug label Dec 5, 2021
@bastelfreak bastelfreak self-assigned this Dec 5, 2021
@codecov
Copy link

codecov bot commented Dec 5, 2021 •
edited
Loading

Codecov Report

Merging #58 (145692f) into master (23195d6) will not change coverage.
The diff coverage is n/a.

❗ Current head 145692f differs from pull request most recent head a04cd2f. Consider uploading reports for the commit a04cd2f to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #58   +/-   ##
=======================================
  Coverage   84.00%   84.00%           
=======================================
  Files           1        1           
  Lines         300      300           
=======================================
  Hits          252      252           
  Misses         48       48
Impacted Files Coverage Δ
lib/beaker/hypervisor/docker.rb 84.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23195d6...a04cd2f. Read the comment docs.

@bastelfreak
Copy link
Member Author

tested this on voxpupuli/puppet-nginx#1492

@bastelfreak
Copy link
Member Author

requires voxpupuli/beaker#1722

ekohl
ekohl requested changes Dec 6, 2021
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I think this needs to depend on a beaker version which includes the patch in the gemspec. Other than that 👍

spec/beaker/hypervisor/docker_spec.rb Outdated Show resolved Hide resolved
ekohl
ekohl requested changes Dec 6, 2021
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I think we should first get the beaker release out and then depend on that in the gemspec.

spec/beaker/hypervisor/docker_spec.rb Outdated Show resolved Hide resolved
@bastelfreak
Copy link
Member Author

I thought about depending on beaker, but I saw this in the Gemfile:
https://github.com/voxpupuli/beaker-docker/blob/master/Gemfile#L16-L20
On purpose beaker-docker doesn't depend on beaker. I don't know why / if it's just to avoid circular dependencies.

@ekohl
Copy link
Member

ekohl commented Dec 6, 2021

Before voxpupuli/beaker@4b77c07#diff-1ebd183c59746ea85d13dbd4792dc98863a406a960f257e38ff0478cc9d905bc at least some things couldn't depend on beaker, but I think that's old and we can now actually depend on it.

@bastelfreak bastelfreak changed the title deduplicate Arch Linux setup deduplicate Arch Linux setup and depend on beaker Dec 6, 2021
@bastelfreak bastelfreak force-pushed the add-archlinux branch 3 times, most recently from 9c8af54 to 96096a9 Compare December 6, 2021 13:36
ekohl
ekohl reviewed Feb 11, 2022
View reviewed changes
lib/beaker/hypervisor/docker.rb Outdated Show resolved Hide resolved
lib/beaker/hypervisor/docker.rb Outdated Show resolved Hide resolved
spec/beaker/hypervisor/docker_spec.rb Show resolved Hide resolved
@bastelfreak
Copy link
Member Author

is it possible that beaker-docker pulls images for CI from dockerhub, but CentOS community publishes to quay. where stream8 exists. but on dockerhub, docker builds centos images and only has eol centos 8, no centos stream 8. I'm trying to figure out where the images are coming from we set in acceptance/config/nodes/hosts.yaml.

@bastelfreak bastelfreak force-pushed the add-archlinux branch 3 times, most recently from f0d986a to 55871e8 Compare February 11, 2022 12:16
@bastelfreak
Copy link
Member Author

@ekohl should be fine now

@ekohl ekohl changed the title deduplicate Arch Linux setup and depend on beaker Deduplicate Arch Linux setup Feb 11, 2022
@bastelfreak bastelfreak force-pushed the add-archlinux branch 2 times, most recently from 2c1ef26 to 6d76b67 Compare February 11, 2022 13:40
@bastelfreak bastelfreak mentioned this pull request Feb 11, 2022
Merged
@bastelfreak bastelfreak force-pushed the add-archlinux branch 2 times, most recently from e6c6262 to 0895314 Compare February 11, 2022 18:51
@bastelfreak bastelfreak changed the title Deduplicate Arch Linux setup Arch Linux: do not install openssh twice Feb 11, 2022
@bastelfreak
Copy link
Member Author

updated the PR title to reflect the current changes

ekohl
ekohl approved these changes Feb 11, 2022
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Thanks! I also like the long options.

@ekohl
Copy link
Member

ekohl commented Feb 11, 2022

Argh, I merged the other PR first and now this conflicts. Sorry.

@bastelfreak
Arch Linux: do not install openssh twice
a04cd2f 
openssh package is already provided by additional_packages(), there is
no need to install it twice.
@bastelfreak bastelfreak merged commit bfdbb46 into voxpupuli:master Feb 11, 2022
@bastelfreak bastelfreak deleted the add-archlinux branch February 11, 2022 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl approved these changes

Assignees

@bastelfreak bastelfreak

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bastelfreak @ekohl