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

fix: Fixes for new pcs and ansible #223

Merged
merged 6 commits into from
Aug 20, 2024
Merged

fix: Fixes for new pcs and ansible #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9a3b8d7
fix: only set resource utilization when it is defined
tomjelinek Aug 9, 2024
2cd253f
test: fix constraints test for pcs-0.12
tomjelinek Aug 9, 2024
8116fae
test: remove jinja2 templating from an assert
tomjelinek Aug 9, 2024
b8a0d17
ci: update internal gitlab CI
tomjelinek Aug 14, 2024
77dae04
refactor: replace deprecated filter password_hash
idevat Aug 14, 2024
7080069
test: fix tests_cib_constraints_create formatting
tomjelinek Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions .gitlab-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ variables:
YAMLLINT_CONFIG: "{extends: .yamllint.yml, ignore: [.github/, .markdownlint.yaml]}"
PYENV_ROOT: /root/pyenvs/ansible

# Ansible 2.9 doesn't work with Python 3.12 shipped in RHEL 10
.all_images: &ALL_IMAGES
- LsrRhel10NextAnsibleCurrent
- LsrRhel10CurrentAnsibleCurrent
- LsrRhel9NextAnsibleCurrent
- LsrRhel9NextAnsible29
- LsrRhel9CurrentAnsibleCurrent
Expand Down Expand Up @@ -38,22 +41,26 @@ variables:

# Linting / code formating that either works the same on all images or using
# the newest linter / formatter is desired
# Rhel9 can be dropped once Rhel10 is stable
.job_generic_linter:
variables:
BASE_IMAGE_NAME: LsrRhel9NextAnsibleCurrent
parallel:
matrix:
- BASE_IMAGE_NAME:
- LsrRhel9NextAnsibleCurrent
- LsrRhel10NextAnsibleCurrent

# If we are running Python 3.11, it won't find installed pcs. Python tests
# only run on RHEL 9 and that ships pcs built for Python 3.9. To overcome
# this, symlink pcs to pyenv.
# If we are running Python version other than what pcs was built for, Python
# won't find installed pcs. To overcome this, symlink pcs to pyenv.
.symlink_pcs_to_pyenv: &symlink_pcs_to_pyenv
- PYTHON_VER=$(python3 -c 'import sys; v=sys.version_info; print(f"{v.major}.{v.minor}")')
- PYENV_PACKAGES="${PYENV_ROOT}/lib/python${PYTHON_VER}/site-packages"
- PCS_DIR=$(rpm -ql pcs | grep 'site-packages/pcs$')
- PCS_PYTHON_VER=$(echo "$PCS_DIR" | sed -e 's@.*/python\([0-9\.]\+\)/.*@\1@')
- PCS_EGG=$(rpm -ql pcs | grep 'site-packages/pcs.*\.egg-info$')
- PCS_EGG_NAME=$(basename "$PCS_EGG")
- if [ "x$PYTHON_VER" != "x3.9" ]; then ln -s "${PCS_DIR}" "${PYENV_PACKAGES}/pcs"; fi
- if [ "x$PYTHON_VER" != "x3.9" ]; then ln -s "${PCS_EGG}" "${PYENV_PACKAGES}/${PCS_EGG_NAME}"; fi
- if [ "x$PYTHON_VER" != "x3.9" ]; then pushd .; cd "${PYENV_PACKAGES}"; rename --verbose "3.9" "$PYTHON_VER" pcs-*.egg-info; popd; fi
- if [ "x$PYTHON_VER" != "x$PCS_PYTHON_VER" ]; then ln -s "${PCS_DIR}" "${PYENV_PACKAGES}/pcs"; fi
- if [ "x$PYTHON_VER" != "x$PCS_PYTHON_VER" ]; then ln -s "${PCS_EGG}" "${PYENV_PACKAGES}/${PCS_EGG_NAME}"; fi
- if [ "x$PYTHON_VER" != "x$PCS_PYTHON_VER" ]; then pushd .; cd "${PYENV_PACKAGES}"; rename --verbose "${PCS_PYTHON_VER}" "${PYTHON_VER}" pcs-*.egg-info; popd; fi
- ls -l "$PYENV_PACKAGES"

.convert_role_to_collection: &convert_role_to_collection
Expand Down Expand Up @@ -92,12 +99,15 @@ ansible_test:
- ansible-test --version
- ansible-test sanity -v --color=yes --truncate=0 --no-redact --coverage --docker # wokeignore:rule=sanity

# TODO enable shellcheck for RHEL 10 once it is available there
shellcheck:
extends: .job_generic_linter
stage: tier0
script:
- shellcheck --version
- shellcheck files/*.sh
rules:
- if: $BASE_IMAGE_NAME !~ /^LsrRhel10.*/

markdownlint:
extends: .job_generic_linter
Expand Down Expand Up @@ -163,7 +173,8 @@ unit_tests:

# tier 1
build_tier1_ci_yml:
extends: .job_generic_linter
variables:
BASE_IMAGE_NAME: LsrRhel9CurrentAnsibleCurrent
stage: tier0
script:
- cd tests
Expand Down
24 changes: 18 additions & 6 deletions tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,26 @@

# The user is created by installing pacemaker packages. We just need to set the
# password.
- name: Set hacluster password
user:
name: hacluster
password: "{{
ha_cluster_hacluster_password | string |
password_hash('sha512', ansible_hostname.replace('-', 'x')[:16]) }}"
- name: Provide a password for the hacluster user
when:
- ha_cluster_hacluster_password | string | length > 0
block:
- name: Generate a password hash
# The arg `-6` means SHA512 based algorithms.
command:
cmd: >-
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to use cmd parameter here

      command: >-
        openssl passwd
          -6
          -salt {{ ansible_hostname.replace('-', 'x') }}
          {{ ha_cluster_hacluster_password | string }}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ansible supports providing the cmd parameter right with command, if cmd is the only parameter that you need to use.

openssl passwd
-6
-salt {{ ansible_hostname.replace('-', 'x') | quote }}
{{ ha_cluster_hacluster_password | string | quote }}
register: __ha_cluster_openssl_call_result
changed_when: false
no_log: true

- name: Set hacluster password
user:
name: hacluster
password: "{{ __ha_cluster_openssl_call_result.stdout }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
password: "{{ __ha_cluster_openssl_call_result.stdout }}"
password: "{{ __ha_cluster_openssl_call_result.stdout }}"
no_log: true

Copy link
Member Author

Choose a reason for hiding this comment

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

I can add no_log, but is it necessary? The password is not shown, the user module hides it even without no_log:

TASK [linux-system-roles.ha_cluster : Set hacluster password] ******************
changed: [localhost] => {
    "append": false,
    "changed": true,
    "comment": "cluster user",
    "group": 189,
    "home": "/var/lib/pacemaker",
    "move_home": false,
    "name": "hacluster",
    "password": "NOT_LOGGING_PASSWORD",
    "shell": "/sbin/nologin",
    "state": "present",
    "uid": 189
}


- name: Configure shell
include_tasks: shell_{{ ha_cluster_pacemaker_shell }}/configure-shell.yml # yamllint disable-line rule:line-length
Expand Down
10 changes: 5 additions & 5 deletions tasks/shell_pcs/pcs-cib-resource-primitive.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@
cmd: >
pcs -f {{ __ha_cluster_tempfile_cib_xml.path | quote }}
-- resource utilization {{ resource.id | quote }}
{% if resource.utilization | d([]) %}
tomjelinek marked this conversation as resolved.
Show resolved Hide resolved
{% for attr in resource.utilization[0].attrs | d([]) %}
{{ attr.name | quote }}={{ attr.value | quote }}
{% endfor %}
{% endif %}
{% for attr in resource.utilization[0].attrs | d([]) %}
{{ attr.name | quote }}={{ attr.value | quote }}
{% endfor %}
# We always need to create CIB to see whether it's the same as what is
# already present in the cluster. However, we don't want to report it as a
# change since the only thing which matters is pushing the resulting CIB to
# the cluster.
check_mode: false
changed_when: not ansible_check_mode
when:
- resource.utilization | d([])
2 changes: 1 addition & 1 deletion tests/tasks/assert_cluster_running.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@
- name: Check cluster is running
assert:
that:
- "'Cluster name: {{ ha_cluster_cluster_name }}' in cluster_status.stdout"
- "'Cluster name: ' ~ ha_cluster_cluster_name in cluster_status.stdout"
- cluster_status.rc == 0
24 changes: 21 additions & 3 deletions tests/tests_cib_constraints_create.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@
when:
- '"pcmk.constraint.config.output-formats" in __test_pcs_capabilities'
vars:
__test_expected_lines: >
__test_expected_lines: |
{
"colocation": [],
"colocation_set": [],
Expand Down Expand Up @@ -728,6 +728,9 @@
"id": "location-d3-rule",
"in_effect": "UNKNOWN",
"options": {
{% if __test_pcs_version is version("0.12", ">=") %}
"boolean-op": "and",
{% endif %}
"score": "INFINITY"
},
"type": "RULE"
Expand Down Expand Up @@ -769,6 +772,9 @@
"id": "location-dd-rule",
"in_effect": "UNKNOWN",
"options": {
{% if __test_pcs_version is version("0.12", ">=") %}
"boolean-op": "and",
{% endif %}
"score": "INFINITY"
},
"type": "RULE"
Expand Down Expand Up @@ -1378,6 +1384,9 @@
"id": "location-d3-4-rule",
"in_effect": "UNKNOWN",
"options": {
{% if __test_pcs_version is version("0.12", ">=") %}
"boolean-op": "and",
{% endif %}
"score": "INFINITY"
},
"type": "RULE"
Expand Down Expand Up @@ -1419,6 +1428,9 @@
"id": "location-dd-4-rule",
"in_effect": "UNKNOWN",
"options": {
{% if __test_pcs_version is version("0.12", ">=") %}
"boolean-op": "and",
{% endif %}
"score": "INFINITY"
},
"type": "RULE"
Expand Down Expand Up @@ -1460,6 +1472,9 @@
"id": "cl9-rule",
"in_effect": "UNKNOWN",
"options": {
{% if __test_pcs_version is version("0.12", ">=") %}
"boolean-op": "and",
{% endif %}
"role": "Unpromoted",
"score": "-47"
},
Expand Down Expand Up @@ -1502,6 +1517,9 @@
"id": "cl10-rule",
"in_effect": "UNKNOWN",
"options": {
{% if __test_pcs_version is version("0.12", ">=") %}
"boolean-op": "and",
{% endif %}
"role": "Unpromoted",
"score": "-47"
},
Expand Down Expand Up @@ -1534,12 +1552,12 @@

- name: Print expected location constraints configuration
debug:
var: __test_expected_lines | from_json
var: __test_expected_lines

- name: Check location constraints configuration
assert:
that:
- __test_pcs_location_config.stdout | from_json == __test_expected_lines | from_json
- __test_pcs_location_config.stdout | from_json == __test_expected_lines

- name: Verify colocation constraints
when:
Expand Down
Loading