Skip to content

[MISC] (8.0/edge) fix units not setting port - #477

Open
paulomach wants to merge 2 commits into
8.0/edgefrom
fix/unit-set-port
Open

[MISC] (8.0/edge) fix units not setting port#477
paulomach wants to merge 2 commits into
8.0/edgefrom
fix/unit-set-port

Conversation

@paulomach

@paulomach paulomach commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

issue

On vm charm, the 3rd unit added to the cluster ports, are not set open.

The code path is two way, at leader deploy time, after the cluster is created; or when a unit joins the cluster in the peer_relation_changed event.
On the later, if a unit back off (e.g. when another unit is joining), later join attempts will happen on other events, missing the set_ports call.
And that's why sometimes on machine we do not see ports being set on juju status.

solution

Add a new set_ports call every time a unit is initialized on the on_start handler.

@paulomach paulomach added the bug Something isn't working as expected label Sep 2, 2026

@sinclert-canonical sinclert-canonical left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code path is two way, at leader deploy time, after the cluster is created; or when a unit joins the cluster in the peer_relation_changed event. On the later, if a unit back off (e.g. when another unit is joining), later join attempts will happen on other events, missing the set_ports call.

Should we move the set_ports call to the start event then (probably at the end of the workload_initialize function)? That way, we would centralize the logic where the ports are set, and will happen regardless of unit role.


If approve, we need to port this to 8.4/edge as well.

@paulomach

Copy link
Copy Markdown
Contributor Author

The code path is two way, at leader deploy time, after the cluster is created; or when a unit joins the cluster in the peer_relation_changed event. On the later, if a unit back off (e.g. when another unit is joining), later join attempts will happen on other events, missing the set_ports call.

Should we move the set_ports call to the start event then (probably at the end of the workload_initialize function)? That way, we would centralize the logic where the ports are set, and will happen regardless of unit role.

If approve, we need to port this to 8.4/edge as well.

on point: 984ce79

Comment thread machines/src/charm.py
except MySQLGetMySQLVersionError:
logger.debug("Fail to get MySQL version")

self.unit.set_ports(3306, 33060)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps worth updating test_on_start unit test (in test_charm.py) to assert this behaviour now?

@Soundarya03 Soundarya03 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left a couple of questions. Thank you!

Comment thread machines/src/charm.py
except MySQLGetMySQLVersionError:
logger.debug("Fail to get MySQL version")

self.unit.set_ports(3306, 33060)

@Soundarya03 Soundarya03 Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is one scenario that comes to mind as I review this change: what about charm upgrade? That is, if we have a unit currently deployed, which is broken with the missing ports issue, refreshing the charm to the latest version with this fix won't help resolve the issue. This is because we've currently plugged on_start and it is further guarded by can_start (so set_ports won't be invoked for already initialized units).
Isn't this relevant for a 8.0 charm specifically because this is already available as a stable release, and is already used in prod?

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

Labels

bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants