Disable DLAMI aws-ubuntu-eni-helper to prevent confict with configure_nw_interface.sh #1485
Conversation
francesco-giordano
force-pushed
the
wip/multi-eni
branch
from
29b3d2c to
be5d0df
Compare
demartinofra
left a comment
demartinofra
left a comment
There was a problem hiding this comment.
Can you please add a changelog entry?
| end | ||
|
|
||
| # Disable DLAMI multi eni helper | ||
| # no further only_if because if the service is not present the action disable do not return error |
| mode "0644" | ||
| end | ||
|
|
||
| # Disable DLAMI multi eni helper |
There was a problem hiding this comment.
Shall we generalize the disable_log4j_patcher recipe to something like disable_services and add this there so that we don't overload the base recipe?
francesco-giordano
force-pushed
the
wip/multi-eni
branch
from
be5d0df to
0504a17
Compare
francesco-giordano
added
skip-changelog-update
and removed
skip-changelog-update
labels
francesco-giordano
force-pushed
the
wip/multi-eni
branch
5 times, most recently
from
d331be7 to
0f48511
Compare
CHANGELOG.md
Outdated
| - selinux-6.0.4 (from selinux-3.1.1) | ||
| - yum-7.4.0 (from yum-6.1.1) | ||
| - yum-epel-4.5.0 (from yum-epel-4.1.2) | ||
| - Disable `aws-ubuntu-eni-helper` service in DLAMI to avoid conflicts with `configure_nw_interface.sh` |
There was a problem hiding this comment.
append something like: when configuring instances with multiple network cards.
Also please add an entry for the MTU related change.
libraries/helpers.rb
Outdated
|
|
||
| # | ||
| # Disable service | ||
| # | ||
| def disable_service(service, platform_families = %i(rhel amazon debian), operations = :disable) | ||
| if platform_family?(platform_families) | ||
| service service do | ||
| action operations | ||
| end | ||
| end | ||
| end | ||
|
|
||
| # | ||
| # Check if a service is disabled | ||
| # | ||
| def service_is_disabled(service, platform_families = %i(rhel amazon debian)) | ||
| if platform_family?(platform_families) | ||
| execute "check #{service} service is disabled" do | ||
| command "systemctl is-enabled #{service} && exit 1 || exit 0" | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
can we move disable_service in an helpers file for the install cookbook and service_is_disabled in an helpers file for the test cookbook?
There was a problem hiding this comment.
minor: I'd rename to is_service_disabled
| # new one is installed in /opt/amazon/efa/bin/ | ||
| # Disable DLAMI multi eni helper | ||
| # no only_if statement because if the service is not present the action disable does not return error | ||
| disable_service('aws-ubuntu-eni-helper', 'debian') |
There was a problem hiding this comment.
would it make sense to mask the service so that if not present at AMI build but installed at a later stage it will stay disabled? also we could do the same for alinux2 in case the service gets added there too
CHANGELOG.md
Outdated
| - selinux-6.0.4 (from selinux-3.1.1) | ||
| - yum-7.4.0 (from yum-6.1.1) | ||
| - yum-epel-4.5.0 (from yum-epel-4.1.2) | ||
| - Disable `aws-ubuntu-eni-helper` service in DLAMI to avoid conflicts with `configure_nw_interface.sh` |
There was a problem hiding this comment.
- I'd write: Disable
aws-ubuntu-eni-helperservice, available in Deep Learning AMIs, to avoid conflicts withconfigure_nw_interface.sh. - minor: missing final period.
| ethernets: | ||
| ${DEVICE_NAME}: | ||
| $STATIC_IP_CONFIG | ||
| mtu: '9001' |
There was a problem hiding this comment.
I'd extend the commit message adding more information about this change. What we were missing and why we're introducing it (with a link to MTU best practices).
The change flollows the best practice described in the official guide https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/network_mtu.html
francesco-giordano
force-pushed
the
wip/multi-eni
branch
from
0f48511 to
8b47a7d
Compare
…_nw_interface.sh Refactor of log4j-cve-2021-44228-hotpatch service disabling
francesco-giordano
force-pushed
the
wip/multi-eni
branch
from
8b47a7d to
6e8619c
Compare
3 participants
Description of changes
Tests
Please review the guidelines for contributing and Pull Request Instructions.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.