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 - 6153] - Propagate the security index intitialized through the peercluster relation #517

Open
wants to merge 3 commits into
base: 2/edge
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/charms/opensearch/v0/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ class PeerClusterRelData(Model):
cm_nodes: List[Node]
credentials: PeerClusterRelDataCredentials
deployment_desc: Optional[DeploymentDescription]
security_index_initialised: bool = False


class PeerClusterRelErrorData(Model):
Expand Down
16 changes: 15 additions & 1 deletion lib/charms/opensearch/v0/opensearch_base_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ def cleanup():
# when "data" node joins -> start cluster-manager via _on_peer_cluster_relation_changed
# cluster-manager notifies "data" node via refresh of peer cluster relation data
# "data" node starts and initializes security index
logger.info(f"Deployment description at on start: {deployment_desc}")
Copy link
Contributor

Choose a reason for hiding this comment

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

The log level here should be debug.

if (
deployment_desc.typ == DeploymentType.MAIN_ORCHESTRATOR
and not deployment_desc.start == StartMode.WITH_GENERATED_ROLES
Expand Down Expand Up @@ -1062,6 +1063,9 @@ def _post_start_init(self, event: _StartOpenSearch): # noqa: C901
"""Initialization post OpenSearch start."""
# initialize the security index if needed (and certs written on disk etc.)
# this happens only on the first data node to join the cluster
logger.debug(
f"is_leader: {self.unit.is_leader()}, security_index_initialised: {self.peers_data.get(Scope.APP, 'security_index_initialised')}, roles: {self.opensearch_peer_cm.deployment_desc().config.roles}, start: {self.opensearch_peer_cm.deployment_desc().start}"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make this a multi-line-statement for better readability.

if (
self.unit.is_leader()
and not self.peers_data.get(Scope.APP, "security_index_initialised")
Expand All @@ -1074,7 +1078,8 @@ def _post_start_init(self, event: _StartOpenSearch): # noqa: C901
admin_secrets = self.secrets.get_object(Scope.APP, CertType.APP_ADMIN.val)
try:
self._initialize_security_index(admin_secrets)
self.peers_data.put(Scope.APP, "security_index_initialised", True)
self.put_or_update_security_index_initialized(event)

except OpenSearchCmdError as e:
logger.debug(f"Error when initializing the security index: {e.out}")
event.defer()
Expand Down Expand Up @@ -1721,6 +1726,15 @@ def handle_joining_data_node(self) -> None:
else:
self._start_opensearch_event.emit(ignore_lock=True)

def put_or_update_security_index_initialized(self, event: EventBase) -> None:
"""Set the security index initialized flag."""
self.peers_data.put(Scope.APP, "security_index_initialised", True)
if self.opensearch_peer_cm.deployment_desc().typ == DeploymentType.MAIN_ORCHESTRATOR:
self.peer_cluster_provider.refresh_relation_data(event)
else:
# notify the main orchestrator that the security index is initialized
self.peer_cluster_requirer.set_security_index_initialised()

@property
def unit_ip(self) -> str:
"""IP address of the current unit."""
Expand Down
38 changes: 37 additions & 1 deletion lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ def _rel_data(
s3=self._s3_credentials(deployment_desc),
),
deployment_desc=deployment_desc,
security_index_initialised=self._get_security_index_initialised(),
)
except OpenSearchHttpError:
return PeerClusterRelErrorData(
Expand Down Expand Up @@ -519,6 +520,22 @@ def _fetch_local_cm_nodes(self, deployment_desc: DeploymentDescription) -> List[
if node.is_cm_eligible() and node.app.id == deployment_desc.app.id
]

def _get_security_index_initialised(self) -> bool:
"""Check if the security index is initialised."""
if self.charm.peers_data.get(Scope.APP, "security_index_initialised", False):
return True

# check all other clusters if they have initialised the security index
all_relation_ids = [
rel.id for rel in self.charm.model.relations[self.relation_name] if len(rel.units) > 0
]

for rel_id in all_relation_ids:
if self.get_from_rel("security_index_initialised", rel_id=rel_id, remote_app=True):
return True
Comment on lines +531 to +538
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is correctly done.


return False


class OpenSearchPeerClusterRequirer(OpenSearchPeerClusterRelation):
"""Peer cluster relation requirer class."""
Expand Down Expand Up @@ -598,6 +615,9 @@ def _on_peer_cluster_relation_changed(self, event: RelationChangedEvent): # noq
# register main and failover cm app names if any
self.charm.peers_data.put_object(Scope.APP, "orchestrators", orchestrators.to_dict())

if data.security_index_initialised:
self.charm.peers_data.put(Scope.APP, "security_index_initialised", True)
Comment on lines +621 to +622
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be happening twice: here and in _set_security_conf. Is this wanted?

I noticed when deploying that the PeerClusterRequirers are updating their opensearch-peers app relation data with security_index_initialised: "True", but the PeerClusterProvider does not. I believe we should add this to the _on_peer_cluster_relation_changed method on the Provider side:

...
if self._get_security_index_initialised():
    self.charm.peers_data.put(Scope.APP, "security_index_initialised", True)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct me if I am wrong. All providers other than the main orchestrator are also requirers of the main orchestrator, correct? That is why I didn't add it for the provider side. The main orchestrator will pick it up and propagate it to the others. The only place the flag might be missing would be the peer databag of the main orchestrator if it was not the one that initialized it. Should we add it there as well for consistency?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that is correct.

The only place the flag might be missing would be the peer databag of the main orchestrator if it was not the one that initialized it.

This is what I meant with my comment, sorry for not being exact enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood. I will add it.


# let the charm know this is an already bootstrapped cluster
self.charm.peers_data.put(Scope.APP, "bootstrapped", True)

Expand Down Expand Up @@ -643,7 +663,8 @@ def _set_security_conf(self, data: PeerClusterRelData) -> None:
)

self.charm.peers_data.put(Scope.APP, "admin_user_initialized", True)
if self.charm.alt_hosts:
logger.debug(f"PeerClusterRelData: {data.to_dict()}")
if data.security_index_initialised:
self.charm.peers_data.put(Scope.APP, "security_index_initialised", True)

if s3_creds := data.credentials.s3:
Expand Down Expand Up @@ -680,6 +701,21 @@ def _orchestrators(

return PeerClusterOrchestrators.from_dict(local_orchestrators)

def set_security_index_initialised(self) -> None:
"""Set the security index as initialised."""
# get the MAIN orchestrator
orchestrators = PeerClusterOrchestrators.from_dict(
self.charm.peers_data.get_object(Scope.APP, "orchestrators") or {}
)

if not orchestrators:
return

# set the security index as initialised in the unit data bag with the main orchestrator
self.put_in_rel(
data={"security_index_initialised": "true"}, rel_id=orchestrators.main_rel_id
)

def _put_current_app(
self, event: RelationEvent, deployment_desc: DeploymentDescription
) -> None:
Expand Down
Loading