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

Added Resize Settings and Resize Operations #551

Merged
merged 1 commit into from
Sep 18, 2023
Merged

Added Resize Settings and Resize Operations #551

merged 1 commit into from
Sep 18, 2023

Conversation

worryg0d
Copy link
Collaborator

@worryg0d worryg0d commented Sep 11, 2023 •
edited
Loading

Done:

  • added resize operations to the cluster data center status. It shows a customer the current resize operations for each data center.
  • added resize settings to the cluster specs
  • added validation of resize settings
  • verbose some messages in the resource update handlers

@worryg0d worryg0d self-assigned this Sep 11, 2023
@worryg0d worryg0d marked this pull request as draft September 11, 2023 11:10
@worryg0d worryg0d force-pushed the issue-514 branch 5 times, most recently from 4f36fc4 to e624381 Compare September 14, 2023 14:59
@worryg0d worryg0d changed the title [WIP] Added Resize Settings and Resize Operations Added Resize Settings and Resize Operations Sep 14, 2023
@worryg0d worryg0d marked this pull request as ready for review September 14, 2023 15:00
@worryg0d worryg0d force-pushed the issue-514 branch 2 times, most recently from 64a1371 to a07fcb3 Compare September 15, 2023 07:36
OleksiienkoMykyta
OleksiienkoMykyta reviewed Sep 15, 2023
View reviewed changes
pkg/models/errors.go Outdated
@@ -61,4 +61,5 @@ var (
ErrPrivateLinkSupportedOnlyForSingleDC = errors.New("private link is only supported for a single data centre")
ErrPrivateLinkSupportedOnlyForAWS = errors.New("private link is supported only for an AWS cloud provider")
ErrImmutableSpec = errors.New("resource specification is immutable")
ErrOnlySingleConcurrentResizeAvailable = errors.New("only 1 concurrent resize is allowed")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should change message from "only 1 concurrent resize is allowed" to "only single concurrent resize is allowed"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed

testisnullus
testisnullus requested changes Sep 15, 2023
View reviewed changes
Makefile Outdated
docker-build: manifests generate test ## Build docker image with the manager.
docker-build: manifests generate #test ## Build docker image with the manager.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert this change please

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

pkg/instaclustr/client.go Outdated
Comment on lines 355 to 356
fmt.Printf("CASSANDRA SPEC UPDATE: %s\n", data)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this code please

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed

@worryg0d worryg0d force-pushed the issue-514 branch 2 times, most recently from 8039cfa to 1c89af8 Compare September 15, 2023 08:15
@testisnullus testisnullus added the enhancement New feature or request label Sep 15, 2023
ribaraka
ribaraka requested changes Sep 18, 2023
View reviewed changes
apis/clusters/v1beta1/cassandra_types.go Outdated
@@ -181,9 +183,10 @@ func (cs *CassandraSpec) HasRestore() bool {
return false
}

func (cs *CassandraSpec) NewDCsUpdate() models.CassandraClusterAPIUpdate {
func (cs *CassandraSpec) ToClusterUpdate() models.CassandraClusterAPIUpdate {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func (cs *CassandraSpec) ToClusterUpdate() models.CassandraClusterAPIUpdate {
func (cs *CassandraSpec) DCsUpdateToInstAPI() models.CassandraClusterAPIUpdate {

The Instaclustr APIv2 has multiple endpoints for updating various fields in a cluster specification. One of them is ClusterSettings. Therefore, I suggest specifically naming the update fields.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

apis/clusters/v1beta1/structs.go Outdated
return iRS
}

type ReplaceOperationInfoV2 struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just call it ReplaceOperation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed

pkg/instaclustr/client.go Outdated
@@ -208,7 +208,7 @@ func (c *Client) GetRedisUser(id string) (*models.RedisUser, error) {
return userRedis, nil
}

func (c *Client) UpdateRedis(id string, r *models.RedisDataCentreUpdate) error {
func (c *Client) UpdateRedis(id string, r *models.RedisUpdate) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

The message with different update fields also applies to this part of the code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@taaraora taaraora merged commit 832dc95 into main Sep 18, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OleksiienkoMykyta OleksiienkoMykyta OleksiienkoMykyta approved these changes

@tengu-alt tengu-alt tengu-alt approved these changes

@testisnullus testisnullus testisnullus approved these changes

@ribaraka ribaraka ribaraka approved these changes

@DoodgeMatvey DoodgeMatvey Awaiting requested review from DoodgeMatvey

Assignees

@worryg0d worryg0d

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@worryg0d @ribaraka @testisnullus @tengu-alt @OleksiienkoMykyta @taaraora