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

CASMAUTO 32 : adding IUF BATS #629

Open
wants to merge 35 commits into
base: release/1.6
Choose a base branch
from
Open

Conversation

Pankhuri-Rajesh
Copy link

@Pankhuri-Rajesh Pankhuri-Rajesh commented Nov 13, 2024

Summary and Scope

Adding the files for IUF test cases which have been automated and are to be made a part of CSM 1.6
Changes-
1. All the goss files for IUF placed under ncn folder. (file name starts with "goss-iuf-")
2. All python scripts are placed under src/csm-testing/tests and lib.
3. iuf_run_setup is the folder containing the dummy-product tar file which will be used for running the tests (mainly IUF stages till prepare-images) and the manifest files which is placed in goss-testing/scripts.
4. New variables-iuf.yaml file added under vars folder.

Issues and Related PRs

Testing

Scripts have been tested by directly installing the modules on the system. Testing with virtual environment is in progress.

Tested on:

  • starlord

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

requirements.txt Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
constraints.txt Outdated Show resolved Hide resolved
goss-testing/scripts/check_iuf_abort.sh Show resolved Hide resolved
goss-testing/scripts/python/check_workflow_template.py Outdated Show resolved Hide resolved
goss-testing/tests/iuf/goss-iuf-activity-abort.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@jacobsalmela jacobsalmela left a comment

Choose a reason for hiding this comment

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

Will any tests need to be skipped on Vshasta? If so, a key should be added for that.

@Srinivas-Anand-HPE Srinivas-Anand-HPE force-pushed the CASMAUTO-32 branch 6 times, most recently from 18b9106 to dd75b9f Compare November 15, 2024 06:51
@Pankhuri-Rajesh
Copy link
Author

Will any tests need to be skipped on Vshasta? If so, a key should be added for that.

all the tests can run on Vshashta

Copy link
Contributor

@mharding-hpe mharding-hpe left a comment

Choose a reason for hiding this comment

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

Have you installed the RPMs on a system and tested to make sure your additions work? I don't see the "testing is complete" checklist item marked in the PR template.

Please do this before requesting a re-review.

constraints.txt Outdated Show resolved Hide resolved
goss-testing/scripts/check_iuf_abort.sh Outdated Show resolved Hide resolved
goss-testing/scripts/workflows/iuf_base_workflow.yaml Outdated Show resolved Hide resolved
goss-testing/scripts/workflows/update_cpc_workflow.yaml Outdated Show resolved Hide resolved
goss-testing/tests/ncn/goss-validate-iuf-manifest.yaml Outdated Show resolved Hide resolved
goss-testing/tests/ncn/goss-validate-iuf-manifest.yaml Outdated Show resolved Hide resolved
requirements.txt Show resolved Hide resolved
requirements.txt Show resolved Hide resolved
goss-testing/tests/ncn/goss-iuf-workflowtemplates.yaml Outdated Show resolved Hide resolved
src/csm_testing/tests/iuf_post_checks/__main__.py Outdated Show resolved Hide resolved
src/csm_testing/tests/iuf_cleanup/__main__.py Outdated Show resolved Hide resolved
src/csm_testing/tests/iuf_cleanup/__main__.py Outdated Show resolved Hide resolved
Copy link
Contributor

@mharding-hpe mharding-hpe left a comment

Choose a reason for hiding this comment

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

My feedback has been addressed. I am marking "request changes" for my review because I don't want to merge until @haasken-hpe approves. It sounds like he has some concerns, based on his comemnt

@haasken-hpe
Copy link
Contributor

Thanks, @mharding-hpe. @Pankhuri-Rajesh set up a meeting with me and the SAT team today, and we talked about my feedback from my earlier comment.

With respect to the inclusion of the sat bootprep input file schema copied from bootprep_schema.yaml in the sat repo, I informed the IUF team that it is possible to obtain this schema file directly from the sat command by running sat bootprep view-schema. Obtaining it in this way has a couple benefits.

  • We will not duplicate the sat bootprep schema file in this repo.
  • The version obtained from sat bootprep view-schema will be the correct version for the version of the sat command on the system.

I also mentioned that sat bootprep run has a --dry-run option that could be useful for performing schema validation. I did point out that this command will do more than just schema validation, and it will require that the variables used in the bootprep file are defined, products referenced in the bootprep file are present in the product catalog, and requested branches of VCS repos exist. If none of that is a problem for this test case, then the sat bootprep run --dry-run command could be used.

In the meeting between the SAT and IUF teams, we also discussed the possible overlap between the test cases being added by the IUF team and those added by the SAT team. I don't believe there will be significant overlap, and we plan to include the IUF team members on reviews for the SAT tests moving forward to ensure awareness of the tests we are adding as well.

We did not talk about the concern I have about the IUF product manifest schema being duplicated in this repository. That's not really my area anymore, but I can say that the cray-nls image does provide a validate command that can be used to validate the schema of a given IUF product manifest file against the schema contained in that cray-nls image. I would suggest that this should be used instead, if possible. I believe that most products are doing this validation already in the build of their product's release distribution tar file. See release-csm.sh in the USS product stream, for example. The function iuf-validate is defined in the release.sh library script in hpc-shastarelm-release. Perhaps these tests can follow that example to do something similar.

@haasken-hpe
Copy link
Contributor

@Pankhuri-Rajesh, please let me know when the feedback about the SAT bootprep schema file has been addressed, and I will take a look at just that piece and approve the PR, so it's clear my feedback has been addressed. I can also take a look at the IUF schema validation when that's addressed, but I'm not the expert there.

@Pankhuri-Rajesh
Copy link
Author

Thanks, @mharding-hpe. @Pankhuri-Rajesh set up a meeting with me and the SAT team today, and we talked about my feedback from my earlier comment.

With respect to the inclusion of the sat bootprep input file schema copied from bootprep_schema.yaml in the sat repo, I informed the IUF team that it is possible to obtain this schema file directly from the sat command by running sat bootprep view-schema. Obtaining it in this way has a couple benefits.

  • We will not duplicate the sat bootprep schema file in this repo.
  • The version obtained from sat bootprep view-schema will be the correct version for the version of the sat command on the system.

I also mentioned that sat bootprep run has a --dry-run option that could be useful for performing schema validation. I did point out that this command will do more than just schema validation, and it will require that the variables used in the bootprep file are defined, products referenced in the bootprep file are present in the product catalog, and requested branches of VCS repos exist. If none of that is a problem for this test case, then the sat bootprep run --dry-run command could be used.

In the meeting between the SAT and IUF teams, we also discussed the possible overlap between the test cases being added by the IUF team and those added by the SAT team. I don't believe there will be significant overlap, and we plan to include the IUF team members on reviews for the SAT tests moving forward to ensure awareness of the tests we are adding as well.

We did not talk about the concern I have about the IUF product manifest schema being duplicated in this repository. That's not really my area anymore, but I can say that the cray-nls image does provide a validate command that can be used to validate the schema of a given IUF product manifest file against the schema contained in that cray-nls image. I would suggest that this should be used instead, if possible. I believe that most products are doing this validation already in the build of their product's release distribution tar file. See release-csm.sh in the USS product stream, for example. The function iuf-validate is defined in the release.sh library script in hpc-shastarelm-release. Perhaps these tests can follow that example to do something similar.

@haasken-hpe I have made the following changes after our discussion-

  1. Instead of placing schema in the repo, "sat bootprep view-schema" is being used to validate the manifest files.
  2. For IUF manifest validation we have replaced the previous method with the podman command which uses cray-nls:0.10.0 image to do the validation. ( referenced from the release.sh library script in hpc-shastarelm-release )
  3. I have also removed the commented lines from management-bootprep file as discussed.
  4. Hence, iuf_schemas folder has been deleted .
  5. I have tested the changed code on noname and it works

Please review the PR changes, and if it's ok then you can approve. Let me know if you have any further follow up on this.

Copy link
Contributor

@haasken-hpe haasken-hpe left a comment

Choose a reason for hiding this comment

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

The bootprep schema validation change looks good, and I'm glad to see the schema files removed from this PR.


try:
cray_nls_image = (
"arti.hpc.amslabs.hpecorp.net/csm-docker-remote/stable/cray-nls:0.10.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this path and version of the image always going to be available where these tests are executed?

f"https://argo.cmn.{cluster_name}.hpc.amslabs.hpecorp.net/",
f"https://vcs.cmn.{cluster_name}.hpc.amslabs.hpecorp.net/",
f"https://nexus.cmn.{cluster_name}.hpc.amslabs.hpecorp.net/",
]
Copy link
Contributor

Choose a reason for hiding this comment

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

hpc.amslabs.hpecorp.net is an internal domain, no customer system will have these values.

One way would be to read the SYSTEM_DOMAIN environment variable if it's available here. Another would be to interrogate Kubernetes for the list of virtual services (kubectl get virtualservice -A)

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.

7 participants