Skip to content

Conversation

@mladjan-gadzic
Copy link
Contributor

What changes were proposed in this pull request?

s3g currently says:

  • /volume/bucket/dir/ exists.
  • /volume/bucket/dir exists.

s3a is expecting:

  • /volume/bucket/dir/ exists.
  • /volume/bucket/dir does not exist.

Proposed is an implementation to resolve the issue and to cover corner cases to support fso folders over s3g by modifying the s3g ObjectEndpoint to check whether the requested path is a directory, if so, to only return ok if path has trailing '/'. This would support s3 Head requests and Get requests. The type of object, where file or directory is determined in the head-object and get-object request through checking the keyInfo (modifying the OzoneKey and OzoneKeyDetails) isFile().

What is the link to the Apache JIRA

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

How was this patch tested?

  • manual
  • unit
  • smoke

@neils-dev neils-dev added the gr label Mar 22, 2023
@mladjan-gadzic mladjan-gadzic marked this pull request as ready for review March 24, 2023 10:52
@kerneltime
Copy link
Contributor

Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @mladjan-gadzic

Overall looks good to me. Left couple of minor comments.

@mladjan-gadzic
Copy link
Contributor Author

@duongkame @hemantk-12 thank you for the review!

Copy link
Contributor

@tanvipenumudy tanvipenumudy left a comment

Choose a reason for hiding this comment

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

nit: Could we consider including a test case that verifies the behavior when attempting to retrieve metadata for a file using a key path that ends with a slash ("/")? The expected behavior is that the request should fail with a 404 status code since the specified key path does not correspond to a directory.

@mladjan-gadzic
Copy link
Contributor Author

@tanvipenumudy thanks for the review! Adding one more test for above-mentioned case makes perfect sense.

// WHEN
final OS3Exception ex =
Assertions.assertThrows(OS3Exception.class,
() -> rest.get(bucketName, keyPath, null, 0, null, body));
Copy link
Contributor

Choose a reason for hiding this comment

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

variable body not found in my env (pull from master branch and merged this pr). I'm not sure where is body defined.

Copy link
Contributor Author

@mladjan-gadzic mladjan-gadzic Jun 5, 2023

Choose a reason for hiding this comment

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

Thanks for looking into it!

It is connected with ac42bf0#diff-fa46c2b691ace246572f975dc2119ab738a19d283ac7848d35e78e8a2709af96L73 change. I merged upstream/master and resolved conflicts. It should be fine now.

@mladjan-gadzic
Copy link
Contributor Author

@kerneltime any chance we can get a motion on this PR? It's been a while and all the comments have been addressed.

@DaveTeng0
Copy link
Contributor

@mladjan-gadzic I saw an integration test (filesystem) failure, could you help take a look? Thanks.

@mladjan-gadzic
Copy link
Contributor Author

@mladjan-gadzic I saw an integration test (filesystem) failure, could you help take a look? Thanks.

@DaveTeng0 thanks for having a look! Failing tests are part of already known issue which has its own Jira https://issues.apache.org/jira/browse/HDDS-8526.

Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

+1 pending CI and conflict resolution.

@mladjan-gadzic Would you resolve the merge conflicts by merging latest master branch into your PR dev branch? Thanks

# Conflicts:
#	hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneKeyDetails.java
#	hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
@mladjan-gadzic
Copy link
Contributor Author

Thank you @smengcl for the review. I merged upstream/master and resolved conflicts. CI on my fork passed. Unfortunatelly I do not have a permission to run CI on upstream. If you could do so, that would be great!

@smengcl smengcl merged commit 68ae858 into apache:master Jun 30, 2023
@smengcl
Copy link
Contributor

smengcl commented Jun 30, 2023

Thanks @duongkame @hemantk-12 @tanvipenumudy @whbing @DaveTeng0 for reviewing this.

vtutrinov pushed a commit to Cyrill/ozone that referenced this pull request Jul 3, 2023
vtutrinov pushed a commit to Cyrill/ozone that referenced this pull request Jul 3, 2023
errose28 added a commit to errose28/ozone that referenced this pull request Jul 10, 2023
* master: (36 commits)
  HDDS-8990. Intermittent timeout waiting on datanode4 9856 to become available (apache#5039)
  Revert "HDDS-7750. Incorrect WRITE ACL check. (apache#4992)"
  HDDS-7750. Incorrect WRITE ACL check. (apache#4992)
  HDDS-8985. Intermittent timeout exiting safe mode in HA secure tests (apache#5033)
  HDDS-8593. Add RootCARotationPoller to CertClient (apache#5030)
  HDDS-7645. Kubernetes check should fail fast if cluster cannot start (apache#5028)
  HDDS-8981. TestRootedOzoneFileSystem runs out of disk space (apache#5029)
  HDDS-8592. Fetch and save all root certificates during service's certificate rotation. (apache#5025)
  HDDS-8981. Disable TestRootedOzoneFileSystem#testSafeMode
  HDDS-8591. Create scheduler to check for new root ca certificates (apache#4961)
  HDDS-8979. error validating kustomization.yaml (apache#5024)
  HDDS-8973. Ozone SCM HA should not allocates duplicate IDs when transferring leadership (apache#5018)
  HDDS-8970. Snapshot Diff should return path relative to bucket root (apache#5015)
  HDDS-8975. Clarify SCM HA auto-bootstrap doc (apache#5021)
  HDDS-8689. Rotate Root CA and Sub CA in SCM. (apache#4943)
  HDDS-8436. Support setSafeMode(), isFileClosed() FileSystem API (apache#4825)
  HDDS-8880. Intermittent fork timeout in TestOMRatisSnapshots (apache#5022)
  HDDS-8962. Ensure docker env is stopped (apache#5011)
  HDDS-7794. [snapshot] SnapshotDiff should throw better error messages for exception handling (apache#5007)
  HDDS-7922. [FSO] S3G folder support fso layout filestatus s3A compatibility (apache#4448)
  ...
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.