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

upcoming: [M3-7536] - Disable fetching buckets with clusters when ObjMultiClusterEnabled flag is enabled. #10282

Merged
merged 10 commits into from
Mar 29, 2024

Conversation

cpathipa
Copy link
Contributor

@cpathipa cpathipa commented Mar 14, 2024

Description 📝

Disables fetching buckets with clusters when ObjMultiClusterEnabled flag is enabled across the CM.

Target release date 🗓️

04/20

How to test 🧪

Prerequisites

(How to setup test environment)

  • Turn on ObjMultiCluster flag and MSW

Verification steps

Turn on the ObjMultiCluster flag and MSW

  • Navigate the http://localhost:3000/object-storage/buckets
  • Ensure no network calls fired to fetch buckets with clusters.
  • Verify the buckets flow. Ensure all buckets flow (Access, SSL/TLS, upload etc.) endpoints should use region instead of cluster.
  • Verify the Access flow. Ensure all access flow endpoints should use region instead of cluster.

Turn off the ObjMultiCluster flag and MSW

  • Verify there is no regression in whole OBJ flow.
  • Verify there is no regression in SearchBar, SearchLanding and PrimaryNav usage.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@cpathipa cpathipa requested a review from a team as a code owner March 14, 2024 13:18
@cpathipa cpathipa requested review from carrillo-erik and jaalah-akamai and removed request for a team March 14, 2024 13:18
@cpathipa cpathipa self-assigned this Mar 14, 2024
@cpathipa cpathipa marked this pull request as draft March 14, 2024 13:18
@cpathipa cpathipa marked this pull request as ready for review March 14, 2024 13:29
Copy link

github-actions bot commented Mar 14, 2024

Coverage Report:
Base Coverage: 81.69%
Current Coverage: 81.71%

Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

ObjMultiCluster flag + MSW on:

  • No network calls fired with clusters to fetch buckets ✅
  • Buckets and Access flows use region instead of cluster

ObjMultiCluster flag and MSW off:

  • No regressions observed in OBJ flow ✅

Compared to develop or even this branch with the feature flag off, when I have the feature flag on and try to navigate to the Search landing page, I get an infinite spinner.

packages/manager/src/components/PrimaryNav/PrimaryNav.tsx Outdated Show resolved Hide resolved
packages/manager/src/queries/objectStorage.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Approving pending Dajahi's feedback is addressed, merge conflicts are resolved, and no one else can replicate the loading issue with the search.

With the feature flag and MSW:

  • Ensured no network calls fired to fetch buckets with clusters*.
  • Verified the buckets flow. Ensure all buckets flow (Access, SSL/TLS, upload etc.) endpoints should use region instead of cluster.
  • Verified the Access flow. Ensure all access flow endpoints should use region instead of cluster.

Without the feature flag and mocks:

  • Verified there is no regression in whole OBJ flow.
  • Verify there is no regression in SearchBar, SearchLanding and PrimaryNav usage.

Compared to develop or even this branch with the feature flag off, when I have the feature flag on and try to navigate to the Search landing page, I get an infinite spinner.

@dwiley-akamai - I couldn't replicate this. Are you still seeing it, and did mean something other than either of the searches I show in this video? This was on this branch with feature flag and mocks off.

Screen.Recording.2024-03-21.at.10.30.17.AM.mov

*One other question b/c I'm not fully familiar with the OMC work: I see we're still fetching the list of object storage clusters on the OBJ landing page, but (as intended) not using clusters for buckets. Do we still need to fetch these all clusters for this feature?

Screenshot 2024-03-21 at 10 01 46 AM

@cpathipa
Copy link
Contributor Author

@mjac0bs Addressed fetching clusters when OMC flag is enabled 44791f3.

@cpathipa cpathipa requested a review from dwiley-akamai March 25, 2024 23:44
@cpathipa cpathipa added the Add'tl Approval Needed Waiting on another approval! label Mar 25, 2024
@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Mar 26, 2024
};

// Utility type to enforce at least one of `clusters` or `regions` to be provided.
export type AtLeastOne<T, U = { [K in keyof T]: Pick<T, K> }> = Partial<T> &
Copy link
Contributor

Choose a reason for hiding this comment

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

The name of this type feels a little weird to me, sounds like a boolean variable name.

@carrillo-erik
Copy link
Contributor

@cpathipa Looks good to me, only one small nitpick with the naming for the AtLeastOne type.

@dwiley-akamai
Copy link
Contributor

dwiley-akamai commented Mar 26, 2024

@dwiley-akamai - I couldn't replicate this. Are you still seeing it, and did mean something other than either of the searches I show in this video? This was on this branch with feature flag and mocks off.

Screen.Recording.2024-03-21.at.10.30.17.AM.mov

@mjac0bs I may have worded it confusingly last time, but I'm observing that behavior when the feature flag is on and mocks off. (for the example below, the account has multiple buckets with "iad" in their names)

Screen.Recording.2024-03-26.at.4.29.59.PM.mov

It shouldn't be an issue when the API is functional, but right now I think useObjectStorageBuckets in SearchLanding.tsx stalls out and areBucketsLoading stays false.

Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

Going ahead and approving since the core of the ticket is complete

packages/manager/src/features/Search/SearchLanding.tsx Outdated Show resolved Hide resolved
@cpathipa
Copy link
Contributor Author

cpathipa commented Mar 27, 2024

@dwiley-akamai I see what you meant. Handled the fetching of buckets with regions from the search landing page more gracefully when the obj flag is on and MSW is off.

SearchLandingPage.mov

@cpathipa cpathipa merged commit 98b5dbf into linode:develop Mar 29, 2024
18 checks passed
bnussman-akamai pushed a commit to bnussman-akamai/manager that referenced this pull request Apr 4, 2024
…MultiClusterEnabled flag is enabled. (linode#10282)

* upcoming: [M3-7536] - Disable fetching buckets with clusters when isObjMultiClusterEnabled flag is enabled.

* Added changeset: Disable fetching buckets with clusters when ObjMultiClusterEnabled flag is enabled.

* code cleanup

* Update packages/manager/.changeset/pr-10282-upcoming-features-1710422378958.md

Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>

* Update regions imports.

* PR - feedback - @mjac0bs  - disable fetching clusters when OMC flag is enabled in buckets landing page.

* PR -feedback - @DevDW - naming convention regionsSupportingObjectStorage

* Organize typeHelpers util and moved AtLeastOne util type to typeHelpers file.

* Update SearchLanding.tsx

---------

Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! OBJ Multi-Cluster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants