Skip to content

[cifmw_cephadm] Support different ceph server and client versions#4012

Closed
fultonj wants to merge 1 commit into
openstack-k8s-operators:mainfrom
fultonj:OSPRH-31757
Closed

[cifmw_cephadm] Support different ceph server and client versions#4012
fultonj wants to merge 1 commit into
openstack-k8s-operators:mainfrom
fultonj:OSPRH-31757

Conversation

@fultonj

@fultonj fultonj commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Add optional parameters to cifmw_cephadm role to support installing cephadm at one version while using client packages at another version. This enables testing scenarios where RHCSv9 clients connect to RHCSv7 servers, or vice versa.

Changes:

  • Add cifmw_cephadm_server_version and cifmw_cephadm_client_version variables (both default to empty string)
  • When both are set and different, temporarily switch to server version repository during cephadm installation, then restore to original cifmw_repo_setup_rhos_release_args
  • Version override logic only activates when both variables are set AND different, ensuring no impact on existing jobs
  • Add warning when client version differs from rhos-release default

The implementation uses regex_replace to swap only the ceph version number in cifmw_repo_setup_rhos_release_args, preserving all other flags like -r for RHEL version.

Example usage for gamma scenario (v7 server + v9 clients):

  cifmw_repo_setup_rhos_release_args: "ceph-9.1-rhel-9 -r 9.4"
  cifmw_cephadm_server_version: "7.1"
  cifmw_cephadm_client_version: "9.1"

This temporarily switches to ceph-7.1 for cephadm installation, then restores to ceph-9.1 for client packages.

Related-Issue: OSPRH-31757

Assisted-By: Claude Sonnet 4.5 noreply@anthropic.com

Add optional parameters to cifmw_cephadm role to support installing
cephadm at one version while using client packages at another version.
This enables testing scenarios where RHCSv9 clients connect to RHCSv7
servers, or vice versa.

Changes:
- Add cifmw_cephadm_server_version and cifmw_cephadm_client_version
  variables (both default to empty string)
- When both are set and different, temporarily switch to server version
  repository during cephadm installation, then restore to original
  cifmw_repo_setup_rhos_release_args
- Version override logic only activates when both variables are set
  AND different, ensuring no impact on existing jobs
- Add warning when client version differs from rhos-release default

The implementation uses regex_replace to swap only the ceph version
number in cifmw_repo_setup_rhos_release_args, preserving all other
flags like -r for RHEL version.

Example usage for gamma scenario (v7 server + v9 clients):
  cifmw_repo_setup_rhos_release_args: "ceph-9.1-rhel-9 -r 9.4"
  cifmw_cephadm_server_version: "7.1"
  cifmw_cephadm_client_version: "9.1"

This temporarily switches to ceph-7.1 for cephadm installation, then
restores to ceph-9.1 for client packages.

Related-Issue: OSPRH-31757

Signed-off-by: John Fulton <fulton@redhat.com>
Assisted-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@fultonj fultonj requested a review from fmount June 24, 2026 00:23
@openshift-ci

openshift-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign akrog for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

block:
- name: Parse version from cifmw_repo_setup_rhos_release_args
ansible.builtin.set_fact:
_cifmw_cephadm_default_version: "{{ cifmw_repo_setup_rhos_release_args | regex_search('ceph-([0-9.]+)') | regex_replace('^ceph-', '') }}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's say that I set _server and _client versions as input that triggers this block, and I reach L32. If somehow cifmw_repo_setup_rhos_release_args is not defined at this level, we fail.
It's good that we fail (in that case there's nothing to recover), but I'm wondering if cifmw_repo_setup_rhos_release_args should be part of the input that triggers this block. In addition, if the regex doesn't match any ceph-x.y, I'm wondering if we want to explicitly fail (it would help for debugging purposes to identify almost immediately where the problem is).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agree on this, we can check if 'ceph-([0-9.]+)' is there at L20. We stop early and we don't let the playbook running with unexpected results!

- "Default: {{ _cifmw_cephadm_default_version }}"
- "Server: {{ cifmw_cephadm_server_version }}"
- "Client: {{ cifmw_cephadm_client_version }}"
- "{{ 'WARNING: Client version differs from default rhos-release version' if (_cifmw_cephadm_default_version | trim) != (cifmw_cephadm_client_version | trim) else 'Client version matches default' }}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we want to print cifmw_repo_setup_rhos_release_args as part of the debug statement?

- "Client: {{ cifmw_cephadm_client_version }}"
- "{{ 'WARNING: Client version differs from default rhos-release version' if (_cifmw_cephadm_default_version | trim) != (cifmw_cephadm_client_version | trim) else 'Client version matches default' }}"

- name: Disable default ceph repository

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm wondering if the Disable tasks that use a regexp are really useful.
If we run rhos-release -x ceph-x.y, the tool ensures that only that repo is available at that time in the system.
This means that we could:

  1. remove L42-47
  2. modify L52 to take the -x option
    (continue with the cephadm install)
  3. remove L77-L83
  4. modify L89 to take -x option

This way we have the same effect and we let the tool explicitly manage the repos.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does rhos-release -x really removes a single repository?
If that's the case, yes, let's not leak here internal details about how rhos-release deploys the repositories.

And if rhos-release didn't do this, I think it should be handled there anyway. We should not manipulate its repositories directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No -x purges everything and then adds what's in the arguments.

# License for the specific language governing permissions and limitations
# under the License.

- name: Check if server/client version override is configured

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Because the new parameters are free form strings, should we guard against bad input, like:

    - name: Validate version override format
      ansible.builtin.assert:
        that:
          - item is match('^[0-9]+\.[0-9]+$')
        fail_msg: >-
          '{{ item }}' must be numeric X.Y format (e.g. '7.1', '9.1')
          or empty string to disable.
      loop:
        - "{{ cifmw_cephadm_server_version | default('7.1') }}"
        - "{{ cifmw_cephadm_client_version | default('8.0') }}"

@tosky

tosky commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

I was wondering whether another set of changes could help, but that would require the help of ci-framework developers. The idea would be to split cifmw_repo_setup_rhos_release_args into a cifmw_repo_setup_rhos_release_ceph and cifmw_repo_setup_rhos_release_os_args or so, and have ci-framework combine them. This way we would have to deal with just a single variable. Moreover, I've seen this pattern in use, and it would be beneficial for every users.

@evallesp

Copy link
Copy Markdown
Contributor

I was wondering whether another set of changes could help, but that would require the help of ci-framework developers. The idea would be to split cifmw_repo_setup_rhos_release_args into a cifmw_repo_setup_rhos_release_ceph and cifmw_repo_setup_rhos_release_os_args or so, and have ci-framework combine them. This way we would have to deal with just a single variable. Moreover, I've seen this pattern in use, and it would be beneficial for every users.

This is a really good idea.

@fultonj

fultonj commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews. I'll prototype another approach which avoids touching rhos-release. If it works, I might close this. I'll be back in touch.

@fultonj

fultonj commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

I'm going to test with #4015 and see if I can acheive the same thing. If so I'll close this.

@fultonj

fultonj commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Obsoleted by #4015

@fultonj fultonj closed this Jun 24, 2026
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.

4 participants