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

aistack_basic test #20427

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yarunachalam
Copy link
Contributor

@yarunachalam yarunachalam commented Oct 18, 2024

Copy link

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

  • Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files.

Copy link
Contributor

@m-dati m-dati left a comment

Choose a reason for hiding this comment

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

Debug lines Not needed, info already printed before

tests/publiccloud/aistack_basic.pm Outdated Show resolved Hide resolved
tests/publiccloud/aistack_basic.pm Outdated Show resolved Hide resolved
tests/publiccloud/aistack_basic.pm Outdated Show resolved Hide resolved
tests/publiccloud/aistack_basic.pm Outdated Show resolved Hide resolved
tests/publiccloud/aistack_basic.pm Outdated Show resolved Hide resolved
@yarunachalam yarunachalam force-pushed the aistack_basic branch 9 times, most recently from 7ff4edd to 0a292a9 Compare October 19, 2024 03:49
@yarunachalam yarunachalam force-pushed the aistack_basic branch 4 times, most recently from 0955912 to e5ff26a Compare November 19, 2024 19:00
Copy link
Contributor

@m-dati m-dati left a comment

Choose a reason for hiding this comment

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

Please double ckeck the repository url.

tests/publiccloud/create_aistack_env.pm Outdated Show resolved Hide resolved
tests/publiccloud/create_aistack_env.pm Show resolved Hide resolved
@m-dati
Copy link
Contributor

m-dati commented Nov 20, 2024

LGTM, after clarified $docker_user_name var. assignment in L#96.
Please add in PR also eventual VR job passed.

m-dati
m-dati previously approved these changes Nov 20, 2024
@m-dati m-dati self-requested a review November 20, 2024 11:11
@m-dati m-dati dismissed their stale review November 20, 2024 11:34

CI failing and still docker_user to fix

@yarunachalam yarunachalam force-pushed the aistack_basic branch 7 times, most recently from 040f0f9 to 13c98d8 Compare November 21, 2024 03:25
Copy link
Member

@foursixnine foursixnine left a comment

Choose a reason for hiding this comment

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

I might have some other nipticks and that's not reason to keep dragging this on my end, aside aside from the data-url thing, looks good.

After adjusting the code, feel free to dismiss my review or request re-review

wrt nvidia

I cannot reuse this code. ` trup_call("pkg install -y --auto-agree-with-licenses nvidia-open-driver-G06-signed-kmp=$driver_version nvidia-compute-utils-G06=$driver_version");

`

I filed: https://progress.opensuse.org/issues/170119

my $git_token = get_var('_SECRET_SSH');
my $ing_ver = get_var('ING_VERSION');
my $local_storage_name = 'local_path_storage.yaml';
my $local_path_url = 'https://raw.githubusercontent.com/os-autoinst/os-autoinst-distri-opensuse/54e38954acd015490d85e7614c00c54db46ed9f7/data/aistack/local_path_storage.yaml';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
my $local_path_url = 'https://raw.githubusercontent.com/os-autoinst/os-autoinst-distri-opensuse/54e38954acd015490d85e7614c00c54db46ed9f7/data/aistack/local_path_storage.yaml';
my $local_path_url = data_url('local_path_storage.yaml');

?

Comment on lines +118 to +120
# local-path-storage.yaml is a copy off https://raw.githubusercontent.com/rancher/local-path-provisioner/v0.0.28/deploy/local-path-storage.yaml
#assert_script_run( "curl " . data_url("aistack/$local_storage_name") . " -o $local_storage_name", 60);
assert_script_run("kubectl apply -f $local_path_url", timeout => 120);
Copy link
Member

Choose a reason for hiding this comment

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

I guess is connected to the above?

@yarunachalam yarunachalam force-pushed the aistack_basic branch 2 times, most recently from 696b32b to a27d6e5 Compare November 21, 2024 18:28
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.

5 participants