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.

Sign up for GitHub

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

Jump to bottom

[Fix] Use cluster list API to determine pinned cluster status #4203

Merged
merged 3 commits into from
Nov 12, 2024
Merged

[Fix] Use cluster list API to determine pinned cluster status #4203

merged 3 commits into from
Nov 12, 2024

Conversation

bgrams
Copy link
Contributor

@bgrams bgrams commented Nov 8, 2024 •
edited by alexott
Loading

Changes

Modify setPinnedStatus to use the clusters list API internally for determining the pinning status of a cluster. The existing implementation using the cluster events API subjects the resource to drift as events expire after a period of time.

Closes #3616

Tests

  • Coverage added to TestResourceClusterCreate and TestResourceClusterCreatePinned.
  • Fixtures modified to mock the necessary API calls in all other relevant tests.
  • TestAccClusterResource_PinAndUnpin acceptance test added
  • make test run locally
  • relevant change in docs/ folder
  • covered with integration tests in internal/acceptance
  • relevant acceptance tests are passing
  • using Go SDK

@bgrams bgrams requested review from a team as code owners November 8, 2024 14:55
@bgrams bgrams requested review from tanmay-db and removed request for a team November 8, 2024 14:55
@bgrams bgrams force-pushed the fix/pinned-cluster branch from 359b580 to c296fec Compare November 8, 2024 14:56
@bgrams bgrams temporarily deployed to test-trigger-is November 11, 2024 11:06 — with GitHub Actions Inactive
alexott
alexott reviewed Nov 11, 2024
View reviewed changes
Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

overall looks good - can we add a simple integration test for it?
Something similar to TestAccClusterResource_WorkloadType in internal/acceptance/cluster_test.go

@github-actions GitHub Actions
Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 4203
  • Commit SHA: 1011fee8905808035dab7aabb0db58c960fdb0e2

Checks will be approved automatically on success.

@bgrams bgrams requested a review from alexott November 11, 2024 15:20
@bgrams bgrams temporarily deployed to test-trigger-is November 11, 2024 15:46 — with GitHub Actions Inactive
@alexott
Copy link
Contributor

alexott commented Nov 11, 2024

@bgrams I'm really thinking if we even need to use any additional APIs. I now see that pinned clusters have the non-emptypinned_by_user_name field. I need to clarify with the clusters team if it's enough...

@bgrams
Copy link
Contributor Author

bgrams commented Nov 11, 2024 •
edited
Loading

Oh, that would be convenient. I don't see this field in the API docs or CLI output. Lmk and I can adapt this PR if needed.

alexott
alexott approved these changes Nov 11, 2024
View reviewed changes
Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

lgtm from the code perspective. Let see if integration tests are passing

@bgrams
Copy link
Contributor Author

bgrams commented Nov 11, 2024

Thanks! Will these run automatically or is manual action required?

@alexott
Copy link
Contributor

alexott commented Nov 11, 2024

I'll run them when we fix one of the environments where tests are currently failing

@eng-dev-ecosystem-bot
Copy link
Collaborator

Test Details: go/deco-tests/11793935188

@alexott alexott enabled auto-merge November 12, 2024 13:18
@alexott alexott added this pull request to the merge queue Nov 12, 2024
Merged via the queue into databricks:main with commit 9178630 Nov 12, 2024
20 checks passed
@bgrams bgrams deleted the fix/pinned-cluster branch November 12, 2024 15:11
hectorcast-db added a commit that referenced this pull request Nov 20, 2024
@hectorcast-db
[Release] Release v1.59.0
c075486 
### New Features and Improvements

 * Add `databricks_mws_network_connectivity_config` and `databricks_mws_network_connectivity_configs` data source ([#3665](#3665)).
 * Add support partitions in policy data sources ([#4181](#4181)).
 * Added `databricks_registered_model_versions` data source ([#4100](#4100)).
 * Update databricks_permissions resource to support vector-search-endpoints ([#4209](#4209)).
 * add `databricks_serving_endpoints` data source ([#4226](#4226)).

### Bug Fixes

 * Add validation for `run_as_mode` in `databricks_query` ([#4233](#4233)).
 * Correct handling of updates for empty comments and `force_destroy` in UC catalog, schema, registered models and volumes ([#4244](#4244)).
 * Fix deletion of dashboard if it was trashed out of band ([#4235](#4235)).
 * Fix waiting for `databricks_vector_search_index` readiness ([#4243](#4243)).
 * Remove single-node validation from interactive clusters ([#4222](#4222)).
 * Remove single-node validation from jobs clusters ([#4216](#4216)).
 * Use cluster list API to determine pinned cluster status ([#4203](#4203)).
 * fix issue cased by setting pause_status in update monitor  ([#4242](#4242)).

### Documentation

 * Clarify workspace provider config ([#4208](#4208)).
 * Update "Databricks Workspace Creator" permissions on gcp-workspace.md ([#4201](#4201)).
 * Update `grants.md` references ([#4246](#4246)).
 * Update description of `group_id` in `databricks_mws_ncc_private_endpoint_rule` ([#4238](#4238)).
 * remove subnet sharing limitation in AWS ([#4239](#4239)).

### Internal Changes

 * Bump Go SDK to latest and generate TF structs ([#4249](#4249)).
 * Mark TestUcAccModelServingProvisionedThroughput as flaky. to be rever… ([#4232](#4232)).
 * Rename resources directory to products in pluginframework ([#4139](#4139)).
 * Revert "mark TestUcAccModelServingProvisionedThroughput as flaky. to … ([#4240](#4240)).
 * Set user agent in some resources implemented in plugin framework ([#4187](#4187)).
 * make `ApplyAndExpectData` work with nested set ([#4237](#4237)).

### Dependency Updates

 * Bump dependencies for Plugin Framework and SDK v2 ([#4215](#4215)).
 * Bump github.com/hashicorp/hcl/v2 from 2.22.0 to 2.23.0 ([#4236](#4236)).
 * Bump github.com/hashicorp/terraform-plugin-testing from 1.10.0 to 1.11.0 ([#4247](#4247)).

### Exporter

 * Add `List` operation for `users` service ([#4204](#4204)).
 * Fix interactive selection of services ([#4245](#4245)).
@hectorcast-db hectorcast-db mentioned this pull request Nov 20, 2024
Merged
hectorcast-db added a commit that referenced this pull request Nov 20, 2024
@hectorcast-db
[Release] Release v1.59.0
81b4e7c 
 * Add `databricks_mws_network_connectivity_config` and `databricks_mws_network_connectivity_configs` data source ([#3665](#3665)).
 * Add support partitions in policy data sources ([#4181](#4181)).
 * Added `databricks_registered_model_versions` data source ([#4100](#4100)).
 * Update databricks_permissions resource to support vector-search-endpoints ([#4209](#4209)).
 * add `databricks_serving_endpoints` data source ([#4226](#4226)).

 * Add validation for `run_as_mode` in `databricks_query` ([#4233](#4233)).
 * Correct handling of updates for empty comments and `force_destroy` in UC catalog, schema, registered models and volumes ([#4244](#4244)).
 * Fix deletion of dashboard if it was trashed out of band ([#4235](#4235)).
 * Fix waiting for `databricks_vector_search_index` readiness ([#4243](#4243)).
 * Remove single-node validation from interactive clusters ([#4222](#4222)).
 * Remove single-node validation from jobs clusters ([#4216](#4216)).
 * Use cluster list API to determine pinned cluster status ([#4203](#4203)).
 * fix issue cased by setting pause_status in update monitor  ([#4242](#4242)).

 * Clarify workspace provider config ([#4208](#4208)).
 * Update "Databricks Workspace Creator" permissions on gcp-workspace.md ([#4201](#4201)).
 * Update `grants.md` references ([#4246](#4246)).
 * Update description of `group_id` in `databricks_mws_ncc_private_endpoint_rule` ([#4238](#4238)).
 * remove subnet sharing limitation in AWS ([#4239](#4239)).

 * Bump Go SDK to latest and generate TF structs ([#4249](#4249)).
 * Mark TestUcAccModelServingProvisionedThroughput as flaky. to be rever… ([#4232](#4232)).
 * Rename resources directory to products in pluginframework ([#4139](#4139)).
 * Revert "mark TestUcAccModelServingProvisionedThroughput as flaky. to … ([#4240](#4240)).
 * Set user agent in some resources implemented in plugin framework ([#4187](#4187)).
 * make `ApplyAndExpectData` work with nested set ([#4237](#4237)).

 * Bump dependencies for Plugin Framework and SDK v2 ([#4215](#4215)).
 * Bump github.com/hashicorp/hcl/v2 from 2.22.0 to 2.23.0 ([#4236](#4236)).
 * Bump github.com/hashicorp/terraform-plugin-testing from 1.10.0 to 1.11.0 ([#4247](#4247)).

 * Add `List` operation for `users` service ([#4204](#4204)).
 * Fix interactive selection of services ([#4245](#4245)).
github-merge-queue bot pushed a commit that referenced this pull request Nov 20, 2024
@hectorcast-db
[Release] Release v1.59.0 (#4252)
68ba26d 
### New Features and Improvements

* Add `databricks_mws_network_connectivity_config` and
`databricks_mws_network_connectivity_configs` data source
([#3665](#3665)).
* Add support partitions in policy data sources
([#4181](#4181)).
* Added `databricks_registered_model_versions` data source
([#4100](#4100)).
* Update databricks_permissions resource to support
vector-search-endpoints
([#4209](#4209)).
* add `databricks_serving_endpoints` data source
([#4226](#4226)).


### Bug Fixes

* Add validation for `run_as_mode` in `databricks_query`
([#4233](#4233)).
* Correct handling of updates for empty comments and `force_destroy` in
UC catalog, schema, registered models and volumes
([#4244](#4244)).
* Fix deletion of dashboard if it was trashed out of band
([#4235](#4235)).
* Fix waiting for `databricks_vector_search_index` readiness
([#4243](#4243)).
* Remove single-node validation from interactive clusters
([#4222](#4222)).
* Remove single-node validation from jobs clusters
([#4216](#4216)).
* Use cluster list API to determine pinned cluster status
([#4203](#4203)).
* fix issue cased by setting pause_status in update monitor
([#4242](#4242)).


### Documentation

* Clarify workspace provider config
([#4208](#4208)).
* Update "Databricks Workspace Creator" permissions on gcp-workspace.md
([#4201](#4201)).
* Update `grants.md` references
([#4246](#4246)).
* Update description of `group_id` in
`databricks_mws_ncc_private_endpoint_rule`
([#4238](#4238)).
* remove subnet sharing limitation in AWS
([#4239](#4239)).


### Internal Changes

* Bump Go SDK to latest and generate TF structs
([#4249](#4249)).
* Mark TestUcAccModelServingProvisionedThroughput as flaky. to be rever…
([#4232](#4232)).
* Rename resources directory to products in pluginframework
([#4139](#4139)).
* Revert "mark TestUcAccModelServingProvisionedThroughput as flaky. to …
([#4240](#4240)).
* Set user agent in some resources implemented in plugin framework
([#4187](#4187)).
* make `ApplyAndExpectData` work with nested set
([#4237](#4237)).


### Dependency Updates

* Bump dependencies for Plugin Framework and SDK v2
([#4215](#4215)).
* Bump github.com/hashicorp/hcl/v2 from 2.22.0 to 2.23.0
([#4236](#4236)).
* Bump github.com/hashicorp/terraform-plugin-testing from 1.10.0 to
1.11.0
([#4247](#4247)).


### Exporter

* Add `List` operation for `users` service
([#4204](#4204)).
* Fix interactive selection of services
([#4245](#4245)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexott alexott alexott approved these changes

@tanmay-db tanmay-db Awaiting requested review from tanmay-db tanmay-db is a code owner automatically assigned from databricks/eng-plat-auto-exp-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ISSUE] Pinned Clusters keep showing as unpinned on subsequent runs.
3 participants
@bgrams @alexott @eng-dev-ecosystem-bot