HDDS-10828. S3 robot tests should cover both OBS and FSO buckets

[sarvekshayr]

What changes were proposed in this pull request?

S3 robot tests cover only OBJECT_STORE buckets as of now. Modified the tests to cover FILE_SYSTEM_OPTMIZED buckets as well.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10828

How was this patch tested?

Tested the patch manually on a docker cluster.
Green CI run - https://github.com/sarvekshayr/ozone/actions/runs/9169163732

[adoroszlai]

Thanks @sarvekshayr for working on this.

We need to be able to run the same Robot tests against Amazon S3, to verify the correctness of the tests.

ozone/hadoop-ozone/dist/src/main/smoketest/s3/s3_compatbility_check.sh

Lines 20 to 21 in 87c3945

	# This script helps to execute S3 robot test against real AWS s3 endpoint
	# To make sure that all of our defined tests cases copies the behavior of AWS

Since OBS and FSO are Ozone-specific features, there is no point in running the tests twice against Amazon S3.

It also introduces unnecessary code duplication.

Acceptance tests cover different kinds of buckets (encrypted bucket, link to other bucket, bucket with EC replication). FSO bucket is already covered the same way:

ozone/hadoop-ozone/dist/src/main/compose/ozone-ha/test.sh

Lines 41 to 45 in 87c3945

	for layout in OBJECT_STORE LEGACY FILE_SYSTEM_OPTIMIZED; do
	execute_robot_test ${SCM} -v BUCKET:${bucket} -v BUCKET_LAYOUT:${layout} -N s3-${layout}-${bucket} ${exclude} s3
	# some tests are independent of the bucket type, only need to be run once
	exclude="--exclude virtual-host --exclude no-bucket-type"
	done

[Tejaskriya]

Thanks for the patch @sarvekshayr. I have a few suggestions, you can find them below in the inline comments.
Also, post these changes, could you please share the link to a green CI run on your fork? It can be considered as a way of testing the patch apart from locally testing with docker.
You could perhaps update the description of the PR in the "How was this patch tested" section.

[Tejaskriya]

Could we improve the naming of variables here? BUCKET1 is very ambiguous, it can be changed to BUCKET_FSO or anything similar.

[tanvipenumudy]

+1

[Tejaskriya]

Same for this variable as well, And all other files having BUCKET1 can be changed to something more sensible

[Tejaskriya]

As we have 2 different key words for creating FSO and OBS buckets, there is no need to pass an argument to these keywords. [Arguments] ${layout}=FILE_SYSTEM_OPTIMIZED can be ommitted.
Instead of passing ${layout} to Create bucket with layout, we can directly mention the layouts ${BUCKET_LAYOUT_OBS} and ${BUCKET_LAYOUT_FSO} respectively.

[Tejaskriya]

The naming of ${BUCKET_LAYOUT} and ${BUCKET_LAYOUT1} can also be changed to ${BUCKET_LAYOUT_OBS} and ${BUCKET_LAYOUT_FSO} respectively.

[tanvipenumudy]

+1

[Tejaskriya]

Referring to the below comment, the BUCKET_LAYOUT variables wouldn't need to be passed here.

[tanvipenumudy]

Thank you @sarvekshayr for working on the patch.

[tanvipenumudy]

Similarly, we can rename the instances of ${BUCKET} and ${BUCKET1} to also include the layout type.

[tanvipenumudy]

I think we can loop (FOR - END) over the buckets ${BUCKET}, ${BUCKET1} and type: OBS/FSO for reducing the redundancy under the individual test cases.

[sarvekshayr]

Closing this PR because the acceptance test already covers all 3 bucket layouts in #6452.
Thank you @adoroszlai for pointing this out and @Tejaskriya and @tanvipenumudy for the review.