Skip to content

[DPE-7316] Additive changes for stereo mode#1648

Open
dragomirp wants to merge 1 commit into16/edgefrom
stereo-mode-additive-code
Open

[DPE-7316] Additive changes for stereo mode#1648
dragomirp wants to merge 1 commit into16/edgefrom
stereo-mode-additive-code

Conversation

@dragomirp
Copy link
Copy Markdown
Contributor

@dragomirp dragomirp commented Apr 24, 2026

Factor out some of the changes for stereo mode unified charm to reduce PR size:

  • Update snaps
  • Factor out async calls for reuse
  • Add RaftController class
  • Add templates for controller service and config
  • Pull systemd charmlib dep

@dragomirp dragomirp added the enhancement New feature, UI change, or workload upgrade label Apr 24, 2026
@github-actions github-actions Bot added the Libraries: OK The charm libs used are OK and in-sync label Apr 24, 2026
Comment thread src/relations/tls.py
old_operator_ca = self.charm.get_secret(UNIT_SCOPE, "old-ca") or ""
internal_ca = self.charm.get_secret(APP_SCOPE, "internal-ca") or ""
return "\n".join((operator_ca, old_operator_ca, internal_ca))
return "\n".join((operator_ca, old_operator_ca, internal_ca)).strip()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will be passed to the watcher, so it's able to call the Patroni REST API with verify flag.

Comment thread src/cluster.py
Comment on lines +249 to +252
if not self._patroni_async_auth:
raise RetryError(
last_attempt=Future.construct(1, Exception("Unable to reach any units"), True)
)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To maintain the current behaviour.

Comment thread src/utils.py
Comment on lines +126 to +128
cafile: str,
auth: BasicAuth | None = None,
verify: bool = True,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Decoupled from the attributes of the Patroni helper class.

Comment thread src/utils.py
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TODO can also be reused by the observer script.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Templated unit for the RAFT observer.

Comment thread templates/watcher.yml.j2
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

RAFT observer config.

Comment thread templates/watcher.yml.j2
{% for partner_addr in partner_addrs -%}
- {{ partner_addr }}:2222
{% endfor %}
self_addr: '{{ self_addr }}:{{ self_port }}'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hard dependency for the observer. We will need to start/stop based on the number of partners.

Comment thread src/raft_controller.py
members: list[str]


def install_service() -> bool:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Out of the class since it's not dependent on existing relations.

Comment thread src/raft_controller.py

# Validate addresses to prevent injection into the systemd unit file
try:
IPv4Address(self_addr)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we only check for IPv4 addresses here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For now, yes, as we still need to properly implement IPV6 (#922 (comment)).

Comment thread src/raft_controller.py
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not really called by anything at the moment so it should be safe to merge.

Comment thread src/raft_controller.py
Comment on lines +93 to +95
except SystemdError as e:
logger.error(f"Failed to reload systemd: {e}")
return False
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should just raise the exception here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that's a good approach, so the user can call juju resolve to retry it.

Comment thread src/raft_controller.py
Comment on lines +153 to +154
create_directory(self.data_dir, 0o700)
create_directory(f"{self.data_dir}/raft", 0o700)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Make sure that necessary directories exist. Had no luck with 600 so far, _daemon_ user didn't get access.

Type=simple
# charmed-postgresql.patroni-raft-controller app lacks network interfaces
# in the snap profile, so run the controller under the patroni app profile.
ExecStart=/snap/bin/charmed-postgresql.patroni-raft-controller {{ config_file }}/%i/patroni-raft.yaml
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ran as root, snap's setpriv shim will deescalate to _daemon_.

Comment thread src/raft_controller.py
logger.error(f"Failed to restart Raft controller: {e}")
return False

def get_status(self, self_port: int, password: str | None) -> ClusterStatus:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will be used for health check part of the spec.

@dragomirp dragomirp marked this pull request as ready for review April 24, 2026 13:47
@dragomirp dragomirp requested a review from a team as a code owner April 24, 2026 13:47
@dragomirp dragomirp requested review from carlcsaposs-canonical, juju-charm-bot, marceloneppel and taurus-forever and removed request for a team April 24, 2026 13:47
@dragomirp dragomirp force-pushed the stereo-mode-additive-code branch from 7fbabe4 to c1ccf83 Compare April 24, 2026 16:03
Comment thread src/cluster.py
)

# TODO we don't know the other cluster's ca
verify = not bool(alternative_endpoints)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Async rel doesn't share CAs. Existing behaviour.

Copy link
Copy Markdown
Member

@marceloneppel marceloneppel left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread src/raft_controller.py
Comment on lines +93 to +95
except SystemdError as e:
logger.error(f"Failed to reload systemd: {e}")
return False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that's a good approach, so the user can call juju resolve to retry it.

Comment thread src/raft_controller.py

# Validate addresses to prevent injection into the systemd unit file
try:
IPv4Address(self_addr)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For now, yes, as we still need to properly implement IPV6 (#922 (comment)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature, UI change, or workload upgrade Libraries: OK The charm libs used are OK and in-sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants