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

sap_general_preconfigure: Fix /etc/hosts check in assert mode #697

Merged
merged 5 commits into from
Apr 26, 2024

Conversation

berndfinger
Copy link
Member

Solves issue #696.

Solves issue sap-linuxlab#696.

Signed-off-by: Bernd Finger <bfinger@redhat.com>
@@ -20,17 +20,18 @@
success_msg: "PASS: The line '{{ sap_general_preconfigure_ip }} {{ sap_general_preconfigure_hostname }}.{{ sap_general_preconfigure_domain }} {{ sap_general_preconfigure_hostname }}' is once in /etc/hosts."
ignore_errors: "{{ sap_general_preconfigure_assert_ignore_errors | d(false) }}"

# We allow more than one line containing sap_general_preconfigure_ip:
Copy link
Contributor

@ja9fuchs ja9fuchs Apr 12, 2024

Choose a reason for hiding this comment

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

Why though? Only the first entry is used. If duplicate entries of the same IP are present it will most likely lead to trouble later and should be reported as a failure.

It's not a healthy configuration to continue the setup with.

register: __sap_general_preconfigure_register_sap_ip_once_assert
ignore_errors: yes
changed_when: no

- name: Assert that there is just one line containing {{ sap_general_preconfigure_ip }} in /etc/hosts
- name: Assert that there is at least one line containing {{ sap_general_preconfigure_ip }} in /etc/hosts
Copy link
Contributor

Choose a reason for hiding this comment

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

As a sysadmin, I would not allow PASS-ing this check if one IP is configured more than once in /etc/hosts.

Copy link
Member Author

Choose a reason for hiding this comment

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

We currently allow more than one line with the same IP address in /etc/hosts in

# We allow more than one line containing sap_ip:
, so we now should discuss the impact of allowing such a configuration in a broader context.

Copy link
Member

@rhmk rhmk Apr 15, 2024

Choose a reason for hiding this comment

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

Hi,
IIRC this was a user request. This user is putting host aliases to separate lines instead of appending to a single line. This was in the very beginning and I am not sure if this has been documented properly.
So I am fine with approving this PR and wait for complaints. Especially because I haven't found any evidence, that this is a common practise in the net.

Copy link
Contributor

@ja9fuchs ja9fuchs left a comment

Choose a reason for hiding this comment

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

Questioning the tolerance of duplicate IP entries in /etc/hosts.
I cannot think of any valid reason for such a configuration to be considered healthy and a basis to run any application installation on.

@berndfinger
Copy link
Member Author

After the reply from @rhmk , let's no longer allow more than one line containing the same IPv4 address in /etc/hosts. This also affects the role sap_maintain_etc_hosts, see #708.

@berndfinger berndfinger self-assigned this Apr 16, 2024
…address

Relates to sap-linuxlab#696.

Signed-off-by: Bernd Finger <bfinger@redhat.com>
This speeds up processing and as a side effect reduces the number
of programming languages.

Relates to sap-linuxlab#696.

Signed-off-by: Bernd Finger <bfinger@redhat.com>
Relates to sap-linuxlab#696.

Signed-off-by: Bernd Finger <bfinger@redhat.com>
... for triggering modified github workflows

Signed-off-by: Bernd Finger <bfinger@redhat.com>
Copy link
Contributor

@ja9fuchs ja9fuchs left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@rhmk rhmk left a comment

Choose a reason for hiding this comment

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

LGTM:

@berndfinger berndfinger merged commit f9f57de into sap-linuxlab:dev Apr 26, 2024
3 of 4 checks passed
@berndfinger berndfinger deleted the issue-696 branch April 26, 2024 09:09
berndfinger added a commit to berndfinger/community.sap_install that referenced this pull request May 13, 2024
Solves issue sap-linuxlab#736.

It's essentially a backout of PR sap-linuxlab#697, especially of commit 1fc1459.

Signed-off-by: Bernd Finger <bfinger@redhat.com>
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.

None yet

3 participants