Fix default EPEL location for CentOS 7. #252
Conversation
|
@codenrhoden: GitHub didn't allow me to request PR reviews from the following users: voor. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: codenrhoden The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
|
Well look at that, it's quietly skipping on AL2 and CentOS7 on AWS as well. EPEL is not actually needed in those environments, so we probably need to consider whether these changes are similarly necessary in AWS as well, since it was an unexpected change. |
@voor If EPEL isn't need for the AMI builds, just adding it back for OVA should be enough, yes? I haven't been doing any AWS builds, but I am no setup to do so. So I could try to do the builds with and without this change. I was definitely thinking that this was only needed for OVAs. But I should check the AMIs! |
|
I'm wrong, it is needed for CentOS7 in AWS because |
|
Building right now with There's no CentOS7 build in Azure yet. |
|
Yes, same change in |
|
Perfect. Let me know if that fixes things (I'm certain it will). That's the same issue I hit when building CentOS OVAs. I'll update this PR with that change when you give it a 👍 . |
A recent change made it possible to set the EPEL location to an empty string to skip installing EPEL. Since this var is now always set by Packer, not setting it makes it be an empty string, and since extra-vars take precedence in Ansible, the net effect is that the var was *always* set to an empty string, even though a default is set in the Ansible role. This patch sets the default value for CentOS 7 back as it was. The addition is only the CentOS 7 file for OVAs, so it does not effect AL2, and if needed in a future CentOS 8 distro, a release specific version can be set there as well.
codenrhoden
force-pushed
the
epel-default
branch
from
3b42df0 to
492f35d
Compare
|
Thanks for looking, @voor! Something something...I need to add PR CI...Something Something... |
|
Yes, we should probably also remove all python2 dependencies from these images. |
Yeah... Was just talking about that today: #249 (comment) Once you are happy, any chance you can |
|
/lgtm |
|
@voor: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
A recent change (#218) made it possible to set the EPEL location to an empty
string to skip installing EPEL. Since this var is now always set by
Packer, not setting it makes it be an empty string, and since extra-vars
take precedence in Ansible, the net effect is that the var was always
set to an empty string, even though a default is set in the Ansible
role.
This patch sets the default value for CentOS 7 back as it was. The
addition is only the CentOS 7 file for OVAs, so it does not effect AL2,
and if needed in a future CentOS 8 distro, a release specific version
can be set there as well.
/assign @detiber
/cc @voor