Skip to content

Conversation

@gabrielcocenza
Copy link
Member

Change the default value to disable redfish. It's expected that when the charm is refreshed, it will disable redfish because this change will triggeer a config-change.

Change the default value to disable redfish. It's expected that
when the charm is refreshed, it will disable redfish because this
change will triggeer a config-change.
jneo8
jneo8 previously approved these changes Sep 4, 2025
Copy link
Contributor

@Pjack Pjack left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@Pjack Pjack left a comment

Choose a reason for hiding this comment

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

It's a break change. We should add more technical details and the reason on this PR.
I also want to hold it for a while, we can manually disable redfish on all running environment to mitigate the impact.

chanchiwai-ray
chanchiwai-ray previously approved these changes Sep 8, 2025
Copy link
Contributor

@chanchiwai-ray chanchiwai-ray left a comment

Choose a reason for hiding this comment

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

(non-blocking) Just curious if you can put this logic in COSAgentProvider. I.e. dynamically set the prom rules arguments.

@gabrielcocenza
Copy link
Member Author

@chanchiwai-ray
I tried to check the arguments of COSAgentProvider, the argument used is the metrics_rules_dir, which is a Path.

class COSAgentProvider(Object):
"""Integration endpoint wrapper for the provider side of the cos_agent interface."""
def __init__(
self,
charm: CharmType,
relation_name: str = DEFAULT_RELATION_NAME,
metrics_endpoints: Optional[List["_MetricsEndpointDict"]] = None,
metrics_rules_dir: str = "./src/prometheus_alert_rules",
logs_rules_dir: str = "./src/loki_alert_rules",
recurse_rules_dirs: bool = False,
log_slots: Optional[List[str]] = None,
dashboard_dirs: Optional[List[str]] = None,
refresh_events: Optional[List] = None,
tracing_protocols: Optional[List[str]] = None,
*,
scrape_configs: Optional[Union[List[dict], Callable]] = None,
):
"""Create a COSAgentProvider instance.
Args:
charm: The `CharmBase` instance that is instantiating this object.
relation_name: The name of the relation to communicate over.
metrics_endpoints: List of endpoints in the form [{"path": path, "port": port}, ...].
This argument is a simplified form of the `scrape_configs`.
The contents of this list will be merged with the contents of `scrape_configs`.
metrics_rules_dir: Directory where the metrics rules are stored.
logs_rules_dir: Directory where the logs rules are stored.
recurse_rules_dirs: Whether to recurse into rule paths.
log_slots: Snap slots to connect to for scraping logs
in the form ["snap-name:slot", ...].
dashboard_dirs: Directory where the dashboards are stored.
refresh_events: List of events on which to refresh relation data.
tracing_protocols: List of protocols that the charm will be using for sending traces.
scrape_configs: List of standard scrape_configs dicts or a callable
that returns the list in case the configs need to be generated dynamically.
The contents of this list will be merged with the contents of `metrics_endpoints`.
"""

I didn't see another way of doing this dynamic without changes in cos_agent lib.

@Pjack Pjack linked an issue Sep 23, 2025 that may be closed by this pull request
3 tasks
Copy link
Contributor

@jneo8 jneo8 left a comment

Choose a reason for hiding this comment

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

Thanks @gabrielcocenza , LGTM

if you already confirm that the alert rules part is working without issue, then I think it's green light to merge it.

@gabrielcocenza gabrielcocenza merged commit bf732ad into canonical:main Sep 23, 2025
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove redfish functionalities.

4 participants