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

[DPE-2701] POC Finish Removal / Scale down of sharded components #265

Merged
merged 73 commits into from
Oct 16, 2023

Conversation

MiaAltieri
Copy link
Contributor

@MiaAltieri MiaAltieri commented Oct 11, 2023

Issues

  1. Juju 3.1.5 has known bug where secrets are removed before the storage detaches
  2. relation broken executes when shards/config-server are scaling down
  3. users can remove shards / config server application and break cluster

Solution

  1. Bump to Juju 3.1.6
  2. Differentiate between scaling down / relation removal in relation-departed
    3.1 when user removes config-server check if currently connected to shards, if so do not detach storage
    3.2 when user removes shard check if currently connected to config-server, if so wait for storage to drain

Testing

# deploy shards + config server
juju deploy ./*charm --config role="config-server" config-server-one 
juju deploy ./*charm --config role="shard" shard-one 
juju deploy ./*charm --config role="shard" shard-two 

# relate shards
juju integrate config-server-one:config-server shard-one:sharding
juju integrate config-server-one:config-server shard-two:sharding

# scale down config-server and verify units don’t get (incorrectly) drained
juju add-unit config-server-one
juju remove-unit  config-server-one/0

# verify updated config
juju ssh config-server-one/1
charmed-mongodb.mongosh <URI>

# scale down  shard and verify units don’t get drained
juju add-unit shard-one
juju remove-unit  shard-one/0

# verify updated config
juju ssh config-server-one/1
charmed-mongodb.mongosh <URI>

# verify removing shard application waits to drain
juju remove-application shard-one/0

# verify removing config server goes into error
juju remove-application config-server

@MiaAltieri MiaAltieri changed the title Test rel scale down [DPE-2701] Finish Removal / Scale down of sharded components Oct 11, 2023
@MiaAltieri MiaAltieri changed the title [DPE-2701] Finish Removal / Scale down of sharded components [DPE-2701] POC Finish Removal / Scale down of sharded components Oct 11, 2023
src/charm.py Outdated Show resolved Hide resolved
@dmitry-ratushnyy
Copy link
Contributor

@MiaAltieri What do you think about updating log level, for logs that are coming before calls to event.defer from logger.info to logger.debug (keeping logger.errors as-is)

Reasoning:
In general, this information is not important to our users and make only sense to check it when something is going wrong

@MiaAltieri
Copy link
Contributor Author

MiaAltieri commented Oct 12, 2023

@dmitry-ratushnyy

What do you think about updating log level, for logs that are coming before calls to event.defer from logger.info to logger.debug (keeping logger.errors as-is)

I am somewhat against this. While I think that we should get a system to differentiate between logger.info and logger.debug, I think the system of changing them before event.defers doesn't perfectly address their differences. But I would be curious what other people think. (TBH I would need to collect my thoughts more carefully here as well before forumulating more of an opinion)

But I do agree with you that we need to create a standardisation for this. Perhaps we can talk to the experts of charming when/where each one is appropriate. But I think the correct place to have this conversation will be in Mattermost and that the changes should be made in a separate PR as we should be consistent in our approach across all code.

@dmitry-ratushnyy
Copy link
Contributor

@MiaAltieri yep, lets have a discussion. I will create a ticket for tracking it

@MiaAltieri MiaAltieri merged commit b19fdb6 into 6/edge Oct 16, 2023
12 of 13 checks passed
@MiaAltieri MiaAltieri deleted the test-rel-scale-down branch October 16, 2023 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants