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

Docker: Fix docker repo for s390x rhel #2707

Merged
merged 1 commit into from
Aug 25, 2022

Conversation

Haroon-Khel
Copy link
Contributor

@Haroon-Khel Haroon-Khel commented Aug 24, 2022

  • commit message has one of the standard prefixes
  • faq.md updated if appropriate
  • other documentation is changed or added (if applicable)
  • playbook changes run through VPC or QPC (if you have access)
  • VPC/QPC not applicable for this PR
  • for inventory.yml changes, bastillion/nagios/jenkins updated accordingly

ref #2700

The former repo does not allow for docker to be installed onto our s390x rhel7 systems. Changing it to https://download.docker.com/linux/rhel/7/s390x/stable/ seems to work

https://awx2.adoptopenjdk.net/#/jobs/playbook/202?job_search=page_size:20;order_by:-finished;not__launch_type:sync

@Haroon-Khel Haroon-Khel requested review from zdtsw and sxa August 24, 2022 17:09
@Haroon-Khel Haroon-Khel changed the title Docker: change docker repo for s390x rhel Docker: Fix docker repo for s390x rhel Aug 24, 2022
@Haroon-Khel
Copy link
Contributor Author

Have there been new changes to the Linter?

Error: no-handler Tasks that run when changed should likely be handlers.
Error: name All names should start with an uppercase letter.

I don't see how these are about my changes

@zdtsw
Copy link
Contributor

zdtsw commented Aug 25, 2022

Have there been new changes to the Linter?

Error: no-handler Tasks that run when changed should likely be handlers.
Error: name All names should start with an uppercase letter.

I don't see how these are about my changes

looks like the last green linter run was on 6.4.0, yours is on 6.5.0

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

Is this ok for RHEL8 too? Do we need a separate entry here now given that this change pretty much makes it identical to the x64 entry?

@sxa
Copy link
Member

sxa commented Aug 25, 2022

Have there been new changes to the Linter?
I don't see how these are about my changes

In that case I suggest raising a separate issue to resolve the linter problems showing after the update

@Haroon-Khel
Copy link
Contributor Author

Is this ok for RHEL8 too? Do we need a separate entry here now given that this change pretty much makes it identical to the x64 entry?

@sxa right now we only install it on rhel/centos 7

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Aug 25, 2022

Our 3 rhel8 machines, all test, do not have docker installed. We have a centos8 machine build-equinix-centos8-armv8-1, but I cant ssh into it (Permission denied (publickey)) to check if we have docker on it.

I can work on getting our playbooks to install docker on rhel/centos8 after this pr is merged

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

Our 3 rhel8 machines, all test, do not have docker installed. We have a centos8 machine build-equinix-centos8-armv8-1, but I cant ssh into it (Permission denied (publickey)) to check if we have docker on it.

I can work on getting our playbooks to install docker on rhel/centos8 after this pr is merged

To be fair, CentOS8 is now EOL so that doesn't matter so much, but it's worth checking it on the Marist RHEL8 system later, but I agree that doesn't need to be part of this PR if it's not causing a problem for the deployments on there. So on that basis I approve, but please create an issue for merging this with the x64 version and seeing if we can install docker on RHEL8 too.

(I can give you access to the equinix Cent8 box if you want it, but I was intending to reinstall it with a supported OS and never use it again as-is)

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.

3 participants