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-4196] Plugin Management Refactor #435

Open
wants to merge 83 commits into
base: 2/edge
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 70 commits
Commits
Show all changes
83 commits
Select commit Hold shift + click to select a range
c3bb8b0
Add support for settings API and avoid restarts as much as possible
phvalguima Jun 9, 2024
577eeee
comment unit tests for now
phvalguima Jun 9, 2024
d85c5fb
Fixes for for plugin_manager
phvalguima Jun 10, 2024
897f8a5
Add handler class to manage the plugin relations
phvalguima Jun 10, 2024
c1b1c0e
Fix if logic
phvalguima Jun 10, 2024
0934290
Fix cached clean-up
phvalguima Jun 10, 2024
66a5a76
Add fixes for test_plugins.py
phvalguima Jun 11, 2024
b53b362
Fix test_charms
phvalguima Jun 11, 2024
eaf503f
Fix check_plugin_manager_ready
phvalguima Jun 11, 2024
a904019
Fixes for large deployments scenario
phvalguima Jun 12, 2024
8b3f15c
Updates following investigation on large deployments
phvalguima Jun 14, 2024
ec15f2e
Move status.clear to be at the end of config_changed
phvalguima Jun 14, 2024
2408c88
Possible large deployments relation fix
phvalguima Jun 15, 2024
61c8753
Fix large deployments
phvalguima Jun 15, 2024
3ef1db7
Convert S3 data struct to dict
phvalguima Jun 15, 2024
74ebaa2
Convert S3 data struct to dict
phvalguima Jun 16, 2024
e2fffbd
Add first batch of review changes
phvalguima Jul 8, 2024
7b9d488
Update status with set instead of clear(..., new_status) and move to …
phvalguima Jul 8, 2024
43306b2
More review changes
phvalguima Jul 8, 2024
4690454
1st stage merge
phvalguima Jul 10, 2024
fb90ce4
First batch of changes to move away from _to_add/_to_del
phvalguima Jul 10, 2024
69d1e20
update unit tests and fix issues
phvalguima Jul 11, 2024
3c326c2
Fixed unit tests for this change
phvalguima Jul 11, 2024
dde3dd8
Latest merge with main
phvalguima Jul 11, 2024
dc3bceb
Update ci.yaml
phvalguima Jul 11, 2024
512dae2
Update plugins and fix missing config exception
phvalguima Jul 16, 2024
bc8ef5a
fix lint
phvalguima Jul 16, 2024
a34a09d
Merge remote-tracking branch 'origin/main' into DPE-4251-plugin-manag…
phvalguima Sep 3, 2024
b11f1c6
Fix lint and unit tests
phvalguima Sep 3, 2024
200cdfa
Move to deployment_desc() instead
phvalguima Sep 3, 2024
751de6a
Fix plugin manager list building for secrets
phvalguima Sep 3, 2024
707193e
Fix plugins and the manager
phvalguima Sep 3, 2024
64b630f
Clarify unknown error in get_service_status()
phvalguima Sep 3, 2024
6d6ee31
Fix backup plugin creation
phvalguima Sep 4, 2024
81b4c44
Allow metaclass to be cleaned with explicit option
phvalguima Sep 4, 2024
7770af8
Updates based on overrides
phvalguima Sep 4, 2024
7862799
Move away from singleton
phvalguima Sep 9, 2024
eeeeb92
Merge remote-tracking branch 'origin' into DPE-4196-improve-plugin-ma…
phvalguima Sep 9, 2024
7b6ef02
Revert the removal of internal_user.yml
phvalguima Sep 9, 2024
4b39c20
Fix unit tests
phvalguima Sep 10, 2024
832ac39
Not forcely restarting works
phvalguima Sep 10, 2024
2ec8363
Minor fixes for unit and integration tests
phvalguima Sep 10, 2024
3b1057a
Fix the convert_values
phvalguima Sep 10, 2024
572163c
Update apply_config to really avoid restarts
phvalguima Sep 10, 2024
12351ef
remove commented lines
phvalguima Sep 10, 2024
24ce089
Renamed method, fixes integration tests
phvalguima Sep 11, 2024
06444d7
Fix status
phvalguima Sep 11, 2024
4672ed7
Fix lint
phvalguima Sep 11, 2024
cfefa5f
Fix status and remove unneeded deferral
phvalguima Sep 11, 2024
a1ae6df
Small fix status setting
phvalguima Sep 11, 2024
e41dfa3
Fix _charm ref
phvalguima Sep 11, 2024
d2fed0f
Add support for managing opensearch plugin model and move secrets to …
phvalguima Sep 12, 2024
20f1b0f
Fix basemodel for s3
phvalguima Sep 12, 2024
793ec30
Fix model creation
phvalguima Sep 12, 2024
6786f0b
Fix the difference between peer and s3 relations in opensearch_plugins
phvalguima Sep 12, 2024
324f49e
Small fix for plugin
phvalguima Sep 12, 2024
c622d04
Add peer cluster observe to non-orchestrator classes
phvalguima Sep 12, 2024
4cb357c
Fixes to comments
phvalguima Sep 12, 2024
f485fed
Fixes for backup module
phvalguima Sep 12, 2024
ac4f6e5
Merge remote-tracking branch 'origin/main' into DPE-4196-improve-plug…
phvalguima Sep 12, 2024
31b8ffd
Remove the peer cluster observe
phvalguima Sep 12, 2024
a7e85c4
Readd peer cluster
phvalguima Sep 12, 2024
feb49b6
Extend timeout
phvalguima Sep 13, 2024
8ccd54d
Fix setup failure message to be shown in app level
phvalguima Sep 13, 2024
b873ce5
Add blocked as the leader unit gets blocked as well
phvalguima Sep 13, 2024
c6bcc64
Reset the comment position
phvalguima Sep 13, 2024
2e0cb02
Merge remote-tracking branch 'origin' into DPE-4196-improve-plugin-ma…
phvalguima Sep 13, 2024
7a9a17f
Fixes post-merge for unit test
phvalguima Sep 13, 2024
bc49845
Remove the _request proxy method and fix unit tests post main merge
phvalguima Sep 13, 2024
1733693
fix lint
phvalguima Sep 13, 2024
182b9f1
Add try/except catches for each request() call or its upstream method…
phvalguima Sep 13, 2024
8324553
Fix the return code for is_idle method
phvalguima Sep 14, 2024
a62f180
Sync docs from Discourse (#451)
github-actions[bot] Sep 26, 2024
c9edade
[DPE-5558] Break CA rotation into integration test groups (#458)
phvalguima Sep 27, 2024
345999d
fix unit tests for TLS in python 3.12
skourta Oct 3, 2024
5ad3cc5
Rebase to 2/edge
phvalguima Oct 9, 2024
fb03837
Merge branch '2/edge' into DPE-4196-improve-plugin-manager
phvalguima Oct 9, 2024
ec07a49
Merge branch 'DPE-5602-tls-unit-tests-are-failing-when-executed-with-…
phvalguima Oct 9, 2024
23a1c24
Change name from to in plugin manager, remove unused methods and …
phvalguima Dec 7, 2024
0cb418d
Remove defer() from the big peer-cluster event
phvalguima Dec 7, 2024
561fa89
[DPE-4196] Rebase the plugin refactor (#520)
phvalguima Dec 13, 2024
c718443
Merge remote-tracking branch 'origin/2/edge' into DPE-4196-improve-pl…
phvalguima Dec 13, 2024
77d9121
Add new alert rules for throttling (#509)
gabrielcocenza Dec 12, 2024
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
6 changes: 5 additions & 1 deletion lib/charms/opensearch/v0/constants_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@
WaitingForOtherUnitServiceOps = "Waiting for other units to complete the ops on their service."
NewIndexRequested = "new index {index} requested"
RestoreInProgress = "Restore in progress..."
PluginConfigCheck = "Plugin configuration check."
BackupSetupStart = "Backup setup started."
BackupConfigureStart = "Configuring backup service..."
BackupInDisabling = "Disabling backup service..."
PluginConfigCheck = "Plugin configuration check."

# Relation Interfaces
ClientRelationName = "opensearch-client"
Expand Down Expand Up @@ -114,3 +114,7 @@

# User-face Backup ID format
OPENSEARCH_BACKUP_ID_FORMAT = "%Y-%m-%dT%H:%M:%SZ"


S3_REPO_BASE_PATH = "/"
S3_RELATION = "s3-credentials"
89 changes: 87 additions & 2 deletions lib/charms/opensearch/v0/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from hashlib import md5
from typing import Any, Dict, List, Literal, Optional

from charms.opensearch.v0.constants_charm import S3_REPO_BASE_PATH
from charms.opensearch.v0.constants_secrets import S3_CREDENTIALS
from charms.opensearch.v0.helper_enums import BaseStrEnum
from pydantic import BaseModel, Field, root_validator, validator
from pydantic.utils import ROOT_KEY
Expand Down Expand Up @@ -258,15 +260,98 @@ def set_promotion_time(cls, values): # noqa: N805
class S3RelDataCredentials(Model):
"""Model class for credentials passed on the PCluster relation."""

access_key: str = Field(alias="access-key")
secret_key: str = Field(alias="secret-key")
access_key: str = Field(alias="access-key", default=None)
Mehdi-Bendriss marked this conversation as resolved.
Show resolved Hide resolved
secret_key: str = Field(alias="secret-key", default=None)

class Config:
"""Model config of this pydantic model."""

allow_population_by_field_name = True


class S3RelData(Model):
"""Model class for the S3 relation data.

This model should receive the data directly from the relation and map it to a model.
"""

bucket: str = Field(default="")
endpoint: str = Field(default="")
region: Optional[str] = None
base_path: Optional[str] = Field(alias="path", default=S3_REPO_BASE_PATH)
protocol: Optional[str] = None
Comment on lines +294 to +298
Copy link
Contributor

Choose a reason for hiding this comment

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

can you explain why so many optionals and default=""?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missing configs from the s3-integrator

storage_class: Optional[str] = Field(alias="storage-class")
tls_ca_chain: Optional[str] = Field(alias="tls-ca-chain")
credentials: S3RelDataCredentials = Field(alias=S3_CREDENTIALS, default=S3RelDataCredentials())
Copy link
Contributor

Choose a reason for hiding this comment

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

can you excplain the default here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missing configs from the s3-integrator


class Config:
"""Model config of this pydantic model."""

allow_population_by_field_name = True

@root_validator
def validate_core_fields(cls, values): # noqa: N805
"""Validate the core fields of the S3 relation data."""
# Do not raise an exception if we are missing all the fields:
if (
not (s3_creds := values.get("credentials"))
and not s3_creds.access_key
and not s3_creds.secret_key
):
raise ValueError("Missing fields: access_key, secret_key")

# Both bucket and endpoint must be set, or not.
if values.get("bucket") and not values.get("endpoint"):
raise ValueError("Missing field: endpoint")
if values.get("endpoint") and not values.get("bucket"):
raise ValueError("Missing field: bucket")
Comment on lines +319 to +323
Copy link
Contributor

Choose a reason for hiding this comment

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

these can be avoided by not making them optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as the above.


return values

@validator(S3_CREDENTIALS, check_fields=False)
def ensure_secret_content(cls, conf: Dict[str, str] | S3RelDataCredentials): # noqa: N805
"""Ensure the secret content is set."""
if not conf:
return None

data = conf
if isinstance(conf, dict):
# We are
data = S3RelDataCredentials.from_dict(conf)

for value in data.dict().values():
if value.startswith("secret://"):
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this needed? we can assume we receive well formatted secrets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a safety check.

raise ValueError(f"The secret content must be passed, received {value} instead")
return data

@staticmethod
def get_endpoint_protocol(endpoint: str) -> str:
"""Returns the protocol based on the endpoint."""
if not endpoint:
return "http"

if endpoint.startswith("http://"):
return "http"
if endpoint.startswith("https://"):
return "https"
return "https"
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
if endpoint.startswith("https://"):
return "https"
return "https"
return "https"

Can you explain why the default https when if no endpoint passed is default to http?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am just following our principle of TLS first. Here, if the user is not explicitly they want to expose their data via HTTP, we enforce the HTTPS.


@classmethod
def from_relation(cls, input_dict: Optional[Dict[str, Any]]):
"""Create a new instance of this class from a json/dict repr.

This method creates a nested S3RelDataCredentials object from the input dict.
"""
if not input_dict:
return cls()

creds = S3RelDataCredentials(**input_dict)
protocol = S3RelData.get_endpoint_protocol(input_dict.get("endpoint"))
return cls.from_dict(
dict(input_dict) | {"protocol": protocol, S3_CREDENTIALS: creds.dict()}
)


class PeerClusterRelDataCredentials(Model):
"""Model class for credentials passed on the PCluster relation."""

Expand Down
Loading
Loading