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

Share k8s worker and control-plane charms #11

Merged
merged 20 commits into from
Feb 5, 2024
Merged

Share k8s worker and control-plane charms #11

merged 20 commits into from
Feb 5, 2024

Conversation

addyess
Copy link
Contributor

@addyess addyess commented Jan 16, 2024

Structure of the charms

The k8s and k8s-worker charms are noticeably tucked into one-another.

└── worker
    ├── charmcraft.yaml
    ├── requirements.txt
    └── k8s
        ├── charmcraft.yaml
        ├── lib
        │   └── charms/...
        ├── requirements.txt
        └── src
            └── charm.py

While unfamiliar to some charm developers, this lets both charms share the exact same src folder. This is accomplished by using the parts.charm.charm-entrypoint value in the worker directory set to k8s/src/charm.py.

What's unique

The unique parts of the charm are what are in each charm's top-level directory:

charmcraft.yaml
config.yaml
actions.yaml
metadata.yaml
requirements.yaml

In order to exclude the k8s exclusive components from the k8s-worker charm, charmcraft will read the worker/.jujuignore file to determine what to leave out of the final charm.

What's not

The shared portions of each charm are within worker/k8s (except for the above mentioned exclusions). This includes shared libraries from worker/k8s/lib, shared source from worker/k8s/src, shared python dependencies from worker/k8s/requirements.txt

How to distinguish which charm code should engage

The charm can distinguish whether it's a control-plane or worker unit by using self.is_worker or self.is_control_plane by querying its metadata.

Why two charms?

Much of the charm's behavior will be identical. They will employ many of the same relations, many of the same resources, configure the same snap, and use many of the same configuration options. One might therefore assume the two should be 1 charm. History with Charmed Kubernetes has proven that having 2 charms split between control-plane and worker has advantages when a relation is split across requires and provides.

Why not use a charm library?

Sharing code between a charm library is a really reasonable idea, there are limitations that a charm library presents:

  • limited to a single file
  • PRs where the library changes doesn't reflect in the secondary charm
  • updating a second charm isn't immediate
    • must upload to charmhub, then download into the secondary charms

How to use two charms in the same code base:

In cases where the charms should diverge the behavior, use a runtime switch to make the decision

if self.is_control_plane:
    # do control-plane only thing
    ...
# do more common things
...
if self.is_worker:
    # do worker only thing
    ...

@addyess addyess requested a review from a team as a code owner January 16, 2024 16:57
@addyess addyess marked this pull request as draft January 16, 2024 19:17
@bschimke95
Copy link
Contributor

What is the status here Adam?

@addyess
Copy link
Contributor Author

addyess commented Feb 2, 2024

What is the status here Adam?

I think i wanna get back into it. Lemme resolve and see if we can get this guy rolling

@addyess addyess force-pushed the akd/code-share branch 2 times, most recently from 1eb3480 to a4e404b Compare February 2, 2024 16:30
@addyess
Copy link
Contributor Author

addyess commented Feb 2, 2024

@addyess addyess marked this pull request as ready for review February 2, 2024 22:38
Copy link
Contributor

@bschimke95 bschimke95 left a comment

Choose a reason for hiding this comment

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

LGTM, only a minor comment

.github/workflows/auto-update-libs-k8s-worker.yaml Outdated Show resolved Hide resolved
charms/k8s/worker/src/charm.py Outdated Show resolved Hide resolved
charms/k8s/charmcraft.yaml Outdated Show resolved Hide resolved
charms/k8s/charmcraft.yaml Outdated Show resolved Hide resolved
Comment on lines 63 to 71
lib:
# move the ./worker/lib path to ./lib since
# charmcraft assumes it to be there once the charm runs
after: [charm]
plugin: nil
source: ./
override-prime: |
rm -rf $CRAFT_PRIME/lib
mv $CRAFT_PRIME/worker/lib $CRAFT_PRIME/lib
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, it seems that charmcraft expects the lib folder to always been at the same path level as charmcraft.yaml and other "charm files" when run on the unit.

This moves the lib/ folder to the top level during the charm build process.

Copy link
Member

@mateoflorido mateoflorido left a comment

Choose a reason for hiding this comment

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

Just a brief comment

charms/k8s/worker/lib/charms/k8s/v0/k8sd_api_manager.py Outdated Show resolved Hide resolved
Copy link
Member

@mateoflorido mateoflorido left a comment

Choose a reason for hiding this comment

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

Great work! LGTM 🎉

@addyess addyess merged commit 9a4556f into main Feb 5, 2024
21 checks passed
@addyess addyess deleted the akd/code-share branch February 5, 2024 22:59
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.

3 participants