Support new distribution-validations and add few enhancements to validation workflow #4447

Divyaasm · 2024-02-13T17:58:34Z

Description

Support Zip and Deb validations.
Move the download_artifacts method from child classes(except docker) to parent class - validation.py.
Changes to validation_args.py to fix the bug [Bug]: Validation run with file-path silently skips tests for incorrect product name #4441
Cluster readiness is checked by requesting the api's instead of adding sleep().
Changes to docker-validation:
1. Add security strong password changes before running the container.
2. Bring up the services opensearchnode1 and opensearchnode2 only when OS is validated, bringup all the services in
  compose file when both OS and OSD are provided.
3. Pull and validate the latest staging docker image in the given version if build number is not provided.

Addresses #4424 and #4423

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2024-02-13T18:13:08Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9ecf30a) 91.55% compared to head (9e94a9e) 91.99%.
Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4447      +/-   ##
==========================================
+ Coverage   91.55%   91.99%   +0.44%     
==========================================
  Files         190      192       +2     
  Lines        6214     6284      +70     
==========================================
+ Hits         5689     5781      +92     
+ Misses        525      503      -22

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jordarlu · 2024-02-15T01:48:59Z

src/validation_workflow/docker/validation_docker.py

@@ -61,7 +61,7 @@ def start_cluster(self) -> bool:
    def validation(self) -> bool:
        # STEP 2 . inspect image digest between opensearchproject(downloaded/local) and opensearchstaging(dockerHub)
        if not self.args.using_staging_artifact_only:
-            self.image_names_list = [self.args.OS_image, self.args.OSD_image]
+            self.image_names_list = ['opensearchproject/' + project for project in self.args.projects]
            self.image_names_list = [x for x in self.image_names_list if (os.path.basename(x) in self.args.projects)]


what does these two lines of self.image_names_list do?

Second this.

I have unassigned the args self.args.OS_image, self.args.OSD_image in args.py and declared a list which stores the images names based on projects=["opensearch", "opensearchdashboards"]
FYI: self.OS_image = 'opensearchproject/opensearch'
self.OSD_image = 'opensearchproject/opensearch-dashboards'

thanks, Divya, I just feel a bit redundant here becasue you constructed the image_names_list from args.projects on the first line of code, and then you do a filtering from the same args.projects on the next line of code. Could you give me an exmaple of expected output from the 2nd line of code assuming the image_names_list has ["opensearchproject/opensearch", "opensearchproject/opensearchdashboards"] at the first line?

I get it now. Sorry missed this line while editing. It's of no use. Will remove it Thanks for pointing.

gaiksaya · 2024-02-15T16:30:00Z

src/validation_workflow/deb/validation_deb.py

+                execute(f'sudo {set_password} dpkg -i {os.path.basename(self.args.file_path.get(project))}', str(self.tmp_dir.path))
+                time.sleep(80)
+        except:
+            raise Exception("Failed to install Opensearch")


I believe the product might be anything? OpenSearch or Dashboards? Recommending to make the error message generic.

src/validation_workflow/rpm/validation_rpm.py

src/validation_workflow/tar/validation_tar.py

gaiksaya · 2024-02-15T16:35:49Z

src/validation_workflow/validation_args.py

+
+        if args.distribution == "docker":
+            if args.osd_build_number != "latest" and "opensearch-dashboards" not in args.projects:
+                raise Exception("Provide opensearch-dashboards in projects argument to validate OSD")


nit:

Suggested change

raise Exception("Provide opensearch-dashboards in projects argument to validate OSD")

raise Exception("Provide opensearch-dashboards in projects argument to validate OpenSearch-Dashboards")

gaiksaya · 2024-02-15T16:38:14Z

Hi Divya, can you please add more details in the PR description explaining what changes are being made. Looks like it is doing a lot more than debian and zip validation.
Also please attach the issue number related to this PR change.

Thanks!

peterzhuamazon

Hi @Divyaasm ,

As @gaiksaya also pointed out here, the PR is huge with a lot of information but little comments or descriptions on what is happening.

I would suggest you separate this PR into two separate ones, lets review and get deb merge and we understand your approach before going back on win zip, as you can also avoid duplicate changes.

You are also missing tests on the PR. With a big PR like this I am hesitate to just merge through without any tests.

Please take a look at the comments and let us know. 😄

Thanks.

src/validation_workflow/deb/validation_deb.py

peterzhuamazon · 2024-02-15T17:19:28Z

src/validation_workflow/deb/validation_deb.py

+                execute(f'sudo {set_password} dpkg -i {os.path.basename(self.args.file_path.get(project))}', str(self.tmp_dir.path))
+                time.sleep(80)
+        except:
+            raise Exception("Failed to install Opensearch")


src/validation_workflow/deb/validation_deb.py

peterzhuamazon · 2024-02-15T17:23:05Z

src/validation_workflow/docker/validation_docker.py

@@ -61,7 +61,7 @@ def start_cluster(self) -> bool:
    def validation(self) -> bool:
        # STEP 2 . inspect image digest between opensearchproject(downloaded/local) and opensearchstaging(dockerHub)
        if not self.args.using_staging_artifact_only:
-            self.image_names_list = [self.args.OS_image, self.args.OSD_image]
+            self.image_names_list = ['opensearchproject/' + project for project in self.args.projects]
            self.image_names_list = [x for x in self.image_names_list if (os.path.basename(x) in self.args.projects)]


Second this.

src/validation_workflow/docker/validation_docker.py

src/validation_workflow/rpm/validation_rpm.py

src/validation_workflow/tar/validation_tar.py

Divyaasm · 2024-02-15T17:33:46Z

Hi Divya, can you please add more details in the PR description explaining what changes are being made. Looks like it is doing a lot more than debian and zip validation. Also please attach the issue number related to this PR change.

Thanks!

Sure Sayali

Signed-off-by: Divya Madala <divyaasm@amazon.com> Add changes to docker Signed-off-by: Divya Madala <divyaasm@amazon.com> Add requested changes Signed-off-by: Divya Madala <divyaasm@amazon.com> Add testcases Signed-off-by: Divya Madala <divyaasm@amazon.com>

Signed-off-by: Divya Madala <divyaasm@amazon.com>

peterzhuamazon

More questions.

src/validation_workflow/validation.py

src/validation_workflow/validation_args.py

src/validation_workflow/validation.py

Signed-off-by: Divya Madala <divyaasm@amazon.com>

peterzhuamazon

Thanks @Divyaasm .

Add a few nitpiks but those are more for the further improvements later.

As for now we just need to address the parameter name change to be more inline with the behavior.

Thanks.

peterzhuamazon · 2024-02-19T21:13:01Z

src/validation_workflow/validation.py

+                f"{self.args.arch}/{self.args.distribution}/dist/{project}/{project}-{self.args.version}-{self.args.platform}-"
+                f"{self.args.arch}.{file_name_suffix}"
+            )
+        if self.args.distribution == "yum":


TODO: APT will be added in a future PR, so probably need to think about how to use generic function instead of having a if statement specifically on yum.

src/validation_workflow/validation.py

src/validation_workflow/validation_args.py

Signed-off-by: Divya Madala <divyaasm@amazon.com>

src/validation_workflow/validation_args.py

peterzhuamazon · 2024-02-20T00:39:52Z

Thanks @Divyaasm for addressing these comments.
Approved now.

Thanks.

peterzhuamazon · 2024-02-20T17:07:22Z

Hi @Divyaasm you need to address the python test failure.

Signed-off-by: Divya Madala <divyaasm@amazon.com>

Divyaasm requested review from dblock, peterzhuamazon, bbarani, gaiksaya, rishabh6788, zelinh, jordarlu, prudhvigodithi and tianleh as code owners February 13, 2024 17:58

github-actions bot added the distinguished-contributor label Feb 13, 2024

Divyaasm force-pushed the adddist branch from 271ce7f to 3253242 Compare February 13, 2024 18:08

Divyaasm force-pushed the adddist branch 5 times, most recently from 949582b to 3261601 Compare February 15, 2024 01:12

jordarlu reviewed Feb 15, 2024

View reviewed changes

gaiksaya reviewed Feb 15, 2024

View reviewed changes

peterzhuamazon requested changes Feb 15, 2024

View reviewed changes

Divyaasm force-pushed the adddist branch 4 times, most recently from 6efda2d to 6817dfc Compare February 16, 2024 01:42

Divyaasm force-pushed the adddist branch 2 times, most recently from 3ba53cf to ce41e97 Compare February 16, 2024 16:36

Fix failing tests

62ffb62

Signed-off-by: Divya Madala <divyaasm@amazon.com>

Divyaasm force-pushed the adddist branch from ce41e97 to 62ffb62 Compare February 16, 2024 16:57

Divyaasm requested review from gaiksaya, peterzhuamazon and jordarlu February 16, 2024 16:58

peterzhuamazon reviewed Feb 16, 2024

View reviewed changes

src/validation_workflow/validation.py Outdated Show resolved Hide resolved

src/validation_workflow/validation_args.py Outdated Show resolved Hide resolved

src/validation_workflow/validation_args.py Outdated Show resolved Hide resolved

peterzhuamazon reviewed Feb 16, 2024

View reviewed changes

src/validation_workflow/validation.py Outdated Show resolved Hide resolved

Divyaasm added 2 commits February 16, 2024 12:50

Add more changes

f0e32e4

Signed-off-by: Divya Madala <divyaasm@amazon.com>

Add more changes

7694e8e

Signed-off-by: Divya Madala <divyaasm@amazon.com>

Divyaasm requested a review from peterzhuamazon February 19, 2024 17:52

Divyaasm changed the title ~~Add Debian and Zip distributions to validation workflow~~ Support new distributions and add few enhancements to validation workflow Feb 19, 2024

Divyaasm changed the title ~~Support new distributions and add few enhancements to validation workflow~~ Support new distribution-validations and add few enhancements to validation workflow Feb 19, 2024

peterzhuamazon reviewed Feb 19, 2024

View reviewed changes

Add requested changes

07582b0

Signed-off-by: Divya Madala <divyaasm@amazon.com>

peterzhuamazon reviewed Feb 20, 2024

View reviewed changes

src/validation_workflow/validation_args.py Outdated Show resolved Hide resolved

peterzhuamazon approved these changes Feb 20, 2024

View reviewed changes

jordarlu approved these changes Feb 20, 2024

View reviewed changes

Update exception statement

9e94a9e

Signed-off-by: Divya Madala <divyaasm@amazon.com>

Divyaasm force-pushed the adddist branch from c970a51 to 9e94a9e Compare February 20, 2024 17:13

Divyaasm merged commit f1f52ad into opensearch-project:main Feb 20, 2024
12 checks passed

Divyaasm deleted the adddist branch February 20, 2024 17:20

This was referenced Feb 20, 2024

[Bug]: Validation run with file-path silently skips tests for incorrect product name #4441

Closed

Add zip and debian distributions to validation workflow #4423

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support new distribution-validations and add few enhancements to validation workflow #4447

Support new distribution-validations and add few enhancements to validation workflow #4447

Divyaasm commented Feb 13, 2024 •

edited

Loading

codecov bot commented Feb 13, 2024 •

edited

Loading

jordarlu Feb 15, 2024 •

edited

Loading

peterzhuamazon Feb 15, 2024

Divyaasm Feb 15, 2024 •

edited

Loading

jordarlu Feb 16, 2024 •

edited

Loading

Divyaasm Feb 16, 2024

gaiksaya Feb 15, 2024

peterzhuamazon Feb 15, 2024

gaiksaya Feb 15, 2024

gaiksaya commented Feb 15, 2024

peterzhuamazon left a comment •

edited

Loading

peterzhuamazon Feb 15, 2024

peterzhuamazon Feb 15, 2024

Divyaasm commented Feb 15, 2024

peterzhuamazon left a comment

peterzhuamazon left a comment

peterzhuamazon Feb 19, 2024

peterzhuamazon commented Feb 20, 2024

peterzhuamazon commented Feb 20, 2024

	raise Exception("Provide opensearch-dashboards in projects argument to validate OSD")
	raise Exception("Provide opensearch-dashboards in projects argument to validate OpenSearch-Dashboards")

Support new distribution-validations and add few enhancements to validation workflow #4447

Support new distribution-validations and add few enhancements to validation workflow #4447

Conversation

Divyaasm commented Feb 13, 2024 • edited Loading

Description

codecov bot commented Feb 13, 2024 • edited Loading

Codecov Report

jordarlu Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Divyaasm Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

jordarlu Feb 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaiksaya commented Feb 15, 2024

peterzhuamazon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Divyaasm commented Feb 15, 2024

peterzhuamazon left a comment

Choose a reason for hiding this comment

peterzhuamazon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterzhuamazon commented Feb 20, 2024

peterzhuamazon commented Feb 20, 2024

Divyaasm commented Feb 13, 2024 •

edited

Loading

codecov bot commented Feb 13, 2024 •

edited

Loading

jordarlu Feb 15, 2024 •

edited

Loading

Divyaasm Feb 15, 2024 •

edited

Loading

jordarlu Feb 16, 2024 •

edited

Loading

peterzhuamazon left a comment •

edited

Loading