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

CRAYSAT-1896: Add support for CFS v2 or v3 to sat bootprep #288

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

haasken-hpe
Copy link
Contributor

@haasken-hpe haasken-hpe commented Nov 15, 2024

Summary and Scope

Add support for using either the CFS v2 or v3 API in sat bootprep based on the value of the command-line option --cfs-version or the config-file option cfs.api_version. Before this change, sat bootprep was hard-coded to always use CFS v2.

Update the InputConfiguration, InputConfigurationLayer, and AdditionalInventory classes to keep a reference to the instance of the version-specific subclass of CFSClientBase, so that they can delegate operations to the appropriate version of the CFS API.

This removes a lot of code that required knowledge of the CFS API and instead uses classes and methods defined in the csm_api_client library, which has already been updated to support both CFS v2 and v3. This should reduce code duplication across sat and csm_api_client.

Some unit tests are no longer needed here because that functionality is tested in the csm_api_client instead. Other unit tests are updated to include the cfs_client instance that needs to be passed into the input layer and configuration objects.

This also makes the playbook property required in the schema because the CFS v3 API requires that the playbook be specified. This is technically a backwards-incompatible schema change because it can invalidate certain bootprep input files if they define layers without playbooks. The default bootprep files contain a playbook for every layer and do not depend on the CFS default playbook value, so this should be a pretty safe and non-disruptive change.

Issues and Related PRs

Testing

Tested on:

  • Local development environment

Test description:

Unit tests only so far.

Still needs to be tested on a real system.

Risks and Mitigations

This has a moderate risk level. It introduces a bootprep input file schema change that can cause files that do not specify playbook to not validate anymore.

It also is a significant code refactor in that it pushes a lot of the CFS-API knowledge out of the bootprep code and offloads it to the csm_api_client library.

These risks can be mitigated by thorough testing to ensure that CFS configuration creation and image customization still works in sat bootprep.

Pull Request Checklist

  • Version number(s) incremented, if applicable
  • Copyrights updated
  • License file intact
  • Target branch correct
  • CHANGELOG.md updated
  • Testing is appropriate and complete, if applicable
  • HPC Product Announcement prepared, if applicable

@haasken-hpe
Copy link
Contributor Author

Keeping this as a draft until I test, but it's a fairly large PR, so feel free to look at it before I finish testing and let me know if you have any questions or concerns.

@haasken-hpe
Copy link
Contributor Author

haasken-hpe commented Nov 15, 2024

It looks like I need to still add support for pagination of resources other than sessions (e.g. configurations). We do a GET of /configurations when we are checking whether any of the configurations that are going to be created by the input file already exist in CFS.

So I need to add paging support for other CFS API resources to python-csm-api-client and then update this code to use that. It should be a pretty straightforward change. I'll work on that next week.

Here's the error I get right now without this support for paging:

Traceback (most recent call last):
  File "/sat/venv/bin/sat", line 8, in <module>
    sys.exit(main())
  File "/sat/venv/lib/python3.9/site-packages/sat/main.py", line 124, in main
    subcommand(args)
  File "/sat/venv/lib/python3.9/site-packages/sat/cli/bootprep/main.py", line 405, in do_bootprep
    do_bootprep_run(schema_validator, args)
  File "/sat/venv/lib/python3.9/site-packages/sat/cli/bootprep/main.py", line 255, in do_bootprep_run
    instance.input_configurations.handle_existing_items(args.overwrite_configs,
  File "/sat/venv/lib/python3.9/site-packages/sat/cli/bootprep/input/base.py", line 456, in handle_existing_items
    existing_items_by_name = self.get_existing_items_by_name()
  File "/sat/venv/lib/python3.9/site-packages/sat/cli/bootprep/input/configuration.py", line 458, in get_existing_items_by_name
    return {
  File "/sat/venv/lib/python3.9/site-packages/sat/cli/bootprep/input/configuration.py", line 459, in <dictcomp>
    configuration.get('name'): [configuration]
AttributeError: 'str' object has no attribute 'get'

This is because a GET to /configurations now returns an object containing the array of configurations under a new key instead of just a plain array.

@haasken-hpe
Copy link
Contributor Author

I also still need to add the --cfs-version command-line option to the bootprep command.

@haasken-hpe haasken-hpe force-pushed the CRAYSAT-1896-bootprep-cfs-v3-support branch from f2c4833 to 1d23089 Compare November 15, 2024 23:00
@haasken-hpe haasken-hpe changed the title CRAYSAT-1841: Add support for CFS v2 or v3 to sat bootprep CRAYSAT-1896: Add support for CFS v2 or v3 to sat bootprep Nov 15, 2024
@haasken-hpe
Copy link
Contributor Author

I appear to have broken the ability to specify "latest" as the product version in a configuration layer as well. I'll have to look into that.

@haasken-hpe
Copy link
Contributor Author

I have fixed the issues noted in my prior comments. That is:

  • Fixed the ability to specify the "latest" value as the product version in a product-based config layer
  • Added the missing --cfs-version option to sat bootprep run
  • Fixed the listing of CFS configurations with the CFS v3 API to handle paging

@haasken-hpe
Copy link
Contributor Author

haasken-hpe commented Nov 18, 2024

Minimal testing performed so far:

ncn-m001:~/haasken/CRAYSAT-1896 # cat bootprep.yaml
---
configurations:
- name: haasken-test
  layers:
  - name: csm-layer
    product:
      name: csm
      version: 1.5.2
    playbook: csm_packages.yml
  - name: csm-layer
    product:
      name: csm
      version: latest
    playbook: site.yml
ncn-m001:~/haasken/CRAYSAT-1896 # sat --loglevel debug  bootprep run  bootprep.yaml --save-files
INFO: Validating given input file bootprep.yaml
WARNING: Schema version specified in input file (1.0.0) has an older minor version than current schema version (1.1.0). Unexpected behavior may occur.
INFO: Input file successfully validated against schema
DEBUG: Loaded auth token from /root/.config/sat/tokens/api_gw_service_nmn_local.vers.json.
DEBUG: Couldn't load the in-cluster config: Service host/port is not set. (proceeding under the assumption that the config should be loaded from the kubeconfig file)
DEBUG: Issuing GET request to URL 'https://api-gw-service-nmn.local/apis/cfs/v3/configurations'
DEBUG: Received response to GET request to URL 'https://api-gw-service-nmn.local/apis/cfs/v3/configurations' with status code: '200': OK
1 CFS configuration already exists with the name haasken-test. Would you like to skip, overwrite, or abort? [skip,overwrite,abort] overwrite
INFO: 1 CFS configuration already exists with the name haasken-test and will be overwritten.
INFO: Creating 1 CFS configuration
INFO: Creating CFS configuration at index 0 with name=haasken-test
INFO: Saving CFS configuration request body to ./cfs-configuration-haasken-test.json
DEBUG: Issuing PUT request to URL 'https://api-gw-service-nmn.local/apis/cfs/v3/configurations/haasken-test'
DEBUG: Received response to PUT request to URL 'https://api-gw-service-nmn.local/apis/cfs/v3/configurations/haasken-test' with status code: '200': OK
INFO: Successfully created CFS configuration at index 0 with name=haasken-test
DEBUG: Given input file did not define any images, so skipping validation.
INFO: Given input did not define any IMS images
INFO: Nothing to create in collection of BOS session templates
################################################################################
CFS configurations
################################################################################
+--------------+
| name         |
+--------------+
| haasken-test |
+--------------+
ncn-m001:~/haasken/CRAYSAT-1896 # cray cfs v3 configurations describe haasken-test
{
  "last_updated": "2024-11-18T23:46:06Z",
  "layers": [
    {
      "clone_url": "https://api-gw-service-nmn.local/vcs/cray/csm-config-management.git",
      "commit": "ac252601e3df13b70d475dff8ee60fb45f1c27b2",
      "name": "csm-layer",
      "playbook": "csm_packages.yml"
    },
    {
      "clone_url": "https://api-gw-service-nmn.local/vcs/cray/csm-config-management.git",
      "commit": "ac252601e3df13b70d475dff8ee60fb45f1c27b2",
      "name": "csm-layer",
      "playbook": "site.yml"
    }
  ],
  "name": "haasken-test"
}

I need to do some more thorough testing, and while doing so, it would probably be nice to think about how we can start creating some automated tests of the sat bootprep functionality to include in the CT framework.

@haasken-hpe haasken-hpe force-pushed the CRAYSAT-1896-bootprep-cfs-v3-support branch 2 times, most recently from 6d6e14a to 054e7bd Compare November 22, 2024 22:08
@haasken-hpe haasken-hpe marked this pull request as ready for review November 25, 2024 17:51
Add support for using either the CFS v2 or v3 API in `sat bootprep`
based on the value of the command-line option `--cfs-version` or the
config-file option `cfs.api_version`. Before this change, `sat bootprep`
was hard-coded to always use CFS v2.

Update the `InputConfiguration`, `InputConfigurationLayer`, and
`AdditionalInventory` classes to keep a reference to the instance of the
version-specific subclass of `CFSClientBase`, so that they can delegate
operations to the appropriate version of the CFS API.

This removes a lot of code that required knowledge of the CFS API and
instead uses classes and methods defined in the `csm_api_client`
library, which has already been updated to support both CFS v2 and v3.
This should reduce code duplication across `sat` and `csm_api_client`.

Some unit tests are no longer needed here because that functionality is
tested in the `csm_api_client` instead. Other unit tests are updated to
include the `cfs_client` instance that needs to be passed into the input
layer and configuration objects.

This also makes the `playbook` property required in the schema because
the CFS v3 API requires that the playbook be specified. This is
technically a backwards-incompatible schema change because it can
invalidate certain bootprep input files if they define layers without
playbooks. The default bootprep files contain a playbook for every
layer and do not depend on the CFS default playbook value, so this
should be a pretty safe and non-disruptive change.

Test Description:
Unit tests only so far.

Still needs to be tested on a real system.
The CFS v3 API returns paged results for the `/components` API resource,
which needs to be handled by the client. Upgrade the version of the
csm_api_client library and use the new `get_configurations` method to
handle the paging.

Test Description:
Not yet tested.
If the value "latest" is specified for the version of a product in a
product-based layer in a CFS configuration in a bootprep input file,
pass in `None` to `from_product_catalog` to properly get the product
catalog data associated with the latest version of the product.

Test Description:
Not tested yet.
@haasken-hpe haasken-hpe force-pushed the CRAYSAT-1896-bootprep-cfs-v3-support branch from 49b5c01 to 1bccea9 Compare November 25, 2024 22:43
@haasken-hpe
Copy link
Contributor Author

Here are my testing results from rocket: https://gist.github.com/haasken-hpe/8cc8455889b7afe729c5b77dcebfc0f7

I have only done limited testing with --cfs-version v2, and I'm just using the v3 default in these tests, but I have done some minimal testing with v2, and I think we can be sure to test this with our automated functional tests we are going to write.

@haasken-hpe
Copy link
Contributor Author

Since I have merged Cray-HPE/python-csm-api-client#47 and have pushed a release/2.3 branch there, we now have a stable build, so I am rebasing this PR to drop the commit that references the unstable repo.

Add the `--cfs-version` command-line option to `sat bootprep run`. This
option is used to select the version of the CFS API to use when creating
CFS configurations or checking for the existence of CFS configurations
when creating other items in the bootprep input file.

Add this option to the man page as well.

Test Description:
Built man page and examined in `man`.

Other testing yet to be performed on a system.
A delay has been observed in `sat bootprep`, and it may be due to the
loading of product catalog data taking longer with the split product
catalog config maps. Add a debug-level log message before and after
loading the product catalog, so we can see what's happening.
Instead of requiring that the playbook be present in the bootprep input
file through the schema, validate it specifically when `sat` is
configured to use the CFS v3 API, since the CFS v2 API does not require
that CFS configuration layers have a playbook. This will allow people to
continue using old bootprep input files that have CFS configuration
layers that do not specify a playbook as long as they use CFS v2. This
is slightly less disruptive than making a schema change.

Test Description:
Unit tests only so far.
The `InputConfigurationLayer` instances stored in the `layers` attribute
of the `InputConfiguration` class were not being validated when the
`validate` method of the `InputConfiguration` was called because this
class did not have any validation methods. Add a validation method that
validates the layers, and make the `InputConfigurationLayer` class
inherit from `Validatable`, so that it has a `validate` method itself.

This has not been added for `AdditionalInventoryLayer` yet because there
are currently no validation methods on that class.

Note that an `index` is added to the constructor of the
`InputConfigurationLayer` class to allow for the `__str__` to
unambiguously refer to the layer within the CFS configuration.

Test Description:
Unit tests have been added to test the `validate` method on the
`InputConfiguration` class.
Add support for Jinja2 template rendering to several more properties in
the `sat bootprep` input file. The following properties now support
Jinja2 templating:

- configurations[].layers[].git.commit
- configurations[].layers[].git.url
- configurations[].layers[].product.commit
- All properties under images[].base.ims
- session_templates[].image.ims.id

Allowing Jinja2 template rendering for these fields is helpful in
writing automated test cases for `sat bootprep` that can run on any
system. These values will vary from system to system, so it's helpful to
pass this information in to the bootprep input file as a variable.

Test Description:
Added unit tests pass. Will also be tested on a system with bootprep
input files that use Jinja templates in these properties.
Add support for Jinja template rendering to the `name`, `url`, `branch`,
and `commit` properties of the `additional_inventory` of a CFS
configuration specified in a bootprep input file.

This is useful for writing automated tests that obtain this information
from a system and pass it in through variables with `--vars-file`.

Test Description:
The added unit tests pass, and it will be tested on rocket as well.
@haasken-hpe haasken-hpe force-pushed the CRAYSAT-1896-bootprep-cfs-v3-support branch from b382efb to c751745 Compare November 26, 2024 17:11
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.

2 participants