-
Notifications
You must be signed in to change notification settings - Fork 592
HDDS-6084. [Multi-Tenant] Handle upgrades to version supporting S3 multi-tenancy #3018
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
Conversation
Change-Id: I165122fb0e324073d260aed847e4aa3c9b41930c
…and TenantCreate. Change-Id: I3b25525fe301a9d6378ca18be7f8dce4ff1c9867
Change-Id: I453f532062b8f43bf9809d9db55b8a6e4f0fe876
Change-Id: Id96a666efa13423b55454d88a761aae5c8ee2ebd
…arg now. Change-Id: If0558ac9e83b751215ccffd6f0822f9d0d124608
Change-Id: I3471aa2ea3353117775253a3c258f9967d2857a1
Change-Id: Icdb73fb568a5c5dc1c490e26f3e988c6e5a7c9bf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @smengcl and adding one of the first OM layout features. Some minor questions/comments in line.
Also, can we add tests for reading/writing s3 keys from an aws client to the upgrade acceptance test to prove we did not break existing s3 functionality? EDIT: I see that's marked as a todo item in the description.
...op-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java
Outdated
Show resolved
Hide resolved
...ne/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeatureAspect.java
Show resolved
Hide resolved
...ne/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeatureAspect.java
Outdated
Show resolved
Hide resolved
...gration-test/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestMultiTenantVolume.java
Show resolved
Hide resolved
...gration-test/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestMultiTenantVolume.java
Outdated
Show resolved
Hide resolved
…rectly on the OM. Change-Id: I84ac02452df899ba6f3b9afa5f2a33390ea8bae5
Change-Id: I3150a4df8a11d8ab97f1ead5e11e9076755b4937
Change-Id: I2bc0b8dbf0c29c904c053e901868150ffa3e3424
Change-Id: Ifed62b7a2ae80c82f15930ceb20149b155835b19
...gration-test/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestMultiTenantVolume.java
Outdated
Show resolved
Hide resolved
…ization. Change-Id: I7fb48bd6086044f00985a8ec820356c7d6575044
…: Python-dotenv could not parse statement starting` Change-Id: Iea465756226bfacc81f328f4082dac1ece95d37a
Change-Id: I08c0156b4468f899822afcb2a73aede7a8ca7e8f
…uster init. Change-Id: I3c017a47f62195e0f921b9f6fa13e42116db51b0
|
Note: the I have triggered a CI in my fork on a branch that has HDDS-6239 applied: |
errose28
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the acceptance tests @smengcl just a few comments on those, everything else looks good.
...gration-test/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestMultiTenantVolume.java
Show resolved
Hide resolved
Change-Id: If126ab0e6a9d4a5e9a57498a662e462d9f59a771
Change-Id: Iacc9a438124a4504a6b78fd7eaa66ab4f20e3b33
|
After 9 (!) retriggers on my CI-2 branch I finally got a green run. Thanks @avijayanhwx for the +1. I will merge this shortly. |
errose28
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @smengcl lgtm as well.
|
Thanks @errose28 and @avijayanhwx for the review and +1's. |
* HDDS-4944: (268 commits) HDDS-6366. [Multi-Tenant] Disallow specifying custom accessId in OzoneManager (apache#3166) HDDS-6275. [Multi-Tenant] Add feature documentation and CLI quick start guide (apache#3095) HDDS-6063. [Multi-Tenant] Use VOLUME_LOCK in read and write requests, and some minor refactoring (apache#3051) HDDS-6214. [Multi-Tenant] Fix KMS Encryption/Decryption (apache#3010) HDDS-6322. Fix Recon getting inccorrect sequenceNumber from OM (apache#3090) HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo… (apache#2785) HDDS-6313. Remove replicas in ContainerStateMap when a container is deleted (apache#3086) HDDS-6186. Selective checks: skip integration check for unit test changes (apache#3061) HDDS-6310. Update json-smart to 2.4.7. (apache#3080) HDDS-6190. Cleanup unnecessary datanode id path checks. (apache#2993) HDDS-6304. Add enforcer to make sure ozone.version equals project.version (apache#3075) HDDS-6309. Update ozone-runner version to 20220212-1 (apache#3079) HDDS-6293. Allow using custom ozone-runner image (apache#3072) HDDS-4126. Freon key generator should support >2GB files. (apache#3054) HDDS-6088. Implement O3FS/OFS getFileChecksum() using file checksum helpers - addendum: fix checkstyle HDDS-6088. Implement O3FS/OFS getFileChecksum() using file checksum helpers. (apache#2935) HDDS-6084. [Multi-Tenant] Handle upgrades to version supporting S3 multi-tenancy (apache#3018) HDDS-6257. Wrong stack trace for S3 errors (apache#3066) HDDS-6278 Improve memory profile for listStatus API call. (apache#3053) HDDS-6285. ozonesecure-mr intermittently failing with timeout downloading packages (apache#3057) ...
| # OM currently only has one layout version. | ||
| _check_om_mlvs 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smengcl @errose28 maybe I'm missing something, but wouldn't this fail if we actually ran upgrade path test?
https://github.com/adoroszlai/hadoop-ozone/runs/6613025379#step:5:738
I think actual OM MLV for 1.2.0 and 1.2.1 is still 0, regardless of this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right.
Though in the latest feature branch it is still 0 (probably fixed when I was resolving conflicts when doing the merge master 3 days ago or some time ago):
Not sure why I bumped it when doing this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, the om version bump in with_new_version_finalized is intended. As after the finalization it should be bumped to the latest version. But in pre_finalized it should still be zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this callback applies when new version is 1.2.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to say when I was merging master to the feature branch just 3 days ago, I had to bump the _check_om_mlvs further to 3 to pass acceptance (misc):
But then I realized this is the one from 1.1.0 to 1.2.0.
Yup so in this case it should still be zero (in 1.2.0).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is due to the upgrade test.sh script was actually testing the latest compiled version rather than the actual 1.2.0 at the time, and didn't have 1.2.1-1.3.0 yet. And yes it should be reverted back to 0 now (if I haven't done that yet).
Thanks for catching this @adoroszlai !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://issues.apache.org/jira/browse/HDDS-6084
TestMultiTenantVolume.hadoop-ozone/dist/src/main/compose/upgrade/upgrades/non-rolling-upgrade/1.1.0-1.2.0/callback.shand friends.