Skip to content
This repository was archived by the owner on Jul 16, 2026. It is now read-only.

Create the initial charm - #2

Merged
weiiwang01 merged 16 commits into
mainfrom
init-charm
Sep 5, 2025
Merged

Create the initial charm#2
weiiwang01 merged 16 commits into
mainfrom
init-charm

Conversation

@weiiwang01

@weiiwang01 weiiwang01 commented Jun 23, 2025

Copy link
Copy Markdown
Collaborator

Overview

Create the initial version of the chrony-client subordinate charm, which installs and configures Chrony on the target machine.

This pull request includes:

  • The chrony-client charm
  • COS integration for the chrony-client charm
  • The initial set of charm documentation
  • Unit and integration tests

Grafana Dashboard:
image

Rationale

Juju Events Changes

Module Changes

Library Changes

Checklist

@github-actions github-actions Bot 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.

license-eye has checked 63 files.

Valid Invalid Ignored Fixed
20 3 40 0
Click to see the invalid file list
  • lib/charms/operator_libs_linux/v0/apt.py
  • lib/charms/operator_libs_linux/v1/systemd.py
  • src/prometheus_alert_rules/chrony.rule
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

Comment thread lib/charms/operator_libs_linux/v0/apt.py
Comment thread lib/charms/operator_libs_linux/v1/systemd.py
@weiiwang01
weiiwang01 marked this pull request as ready for review June 23, 2025 18:48
@weiiwang01
weiiwang01 requested a review from erinecon June 23, 2025 18:49

@erinecon erinecon 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.

Thanks for the initial documentation set!

Comment thread docs/explanation/charm-architecture.md Outdated
Comment thread docs/explanation/charm-architecture.md Outdated
title Component diagram for Chrony client charm

System_Boundary(vm, "VM machine") {
Container(principal, "principal charm")

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.

For my understanding, is the "principal charm" a Chrony charm?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The principal charm can be any machine charm.

The chrony-client charm attaches to any machine charm and configures the NTP client on that machine. And target machine charm is called the principal charm in juju terms

Comment thread docs/explanation/charm-architecture.md Outdated
Comment thread docs/explanation/charm-architecture.md Outdated
Comment thread docs/how-to/contribute.md Outdated
Comment thread docs/reference/metrics.md
Comment thread docs/tutorial.md Outdated
Comment thread docs/tutorial.md Outdated
Comment thread docs/tutorial.md
Comment thread docs/tutorial.md
Comment thread .licenserc.yaml
Comment thread README.md Outdated
Comment thread charmcraft.yaml Outdated
Comment thread src/chrony.py
Comment thread tests/unit/test_charm.py

@erinecon erinecon 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.

Thanks for implementing my suggestions! Two more nitpicks to consider

Comment thread docs/explanation/charm-architecture.md Outdated
Comment thread docs/explanation/charm-architecture.md Outdated
@erinecon erinecon added the documentation Improvements or additions to documentation label Jul 7, 2025

@erinecon erinecon 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 documentation is looking great! Here's another round of comments from me (mostly updating relate to integrate).

Comment thread docs/explanation/charm-architecture.md Outdated
Comment thread docs/explanation/charm-architecture.md
Comment thread docs/explanation/security.md Outdated
Comment thread docs/how-to/integrate-with-cos.md Outdated
Comment thread docs/how-to/integrate-with-cos.md Outdated
Comment thread docs/how-to/integrate-with-cos.md Outdated
Comment thread docs/how-to/integrate-with-cos.md Outdated
Comment thread docs/how-to/integrate-with-cos.md Outdated
Comment thread docs/how-to/upgrade.md Outdated
Comment thread docs/tutorial.md Outdated
Co-authored-by: Erin Conley <erin.conley@canonical.com>

@dimaqq dimaqq left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry, only a partial review so far

Comment thread docs/explanation/charm-architecture.md Outdated
Comment thread docs/reference/metrics.md Outdated

@erinecon erinecon 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.

A few more nitpicks I noticed, but it's a 👍 on the documentation side of things. Thanks!

Comment thread docs/explanation/charm-architecture.md Outdated
Comment thread docs/explanation/charm-architecture.md Outdated
Comment thread docs/explanation/charm-architecture.md Outdated
Comment thread docs/explanation/security.md Outdated
Comment thread docs/how-to/integrate-with-cos.md Outdated
Comment thread .woke.yaml
Comment thread docs/changelog.md
@yhaliaw

yhaliaw commented Jul 24, 2025

Copy link
Copy Markdown
Contributor

@weiiwang01 Can a screenshot of the Grafana dashboard be included here?

@yhaliaw
yhaliaw requested a review from gregory-schiano July 24, 2025 03:30
@weiiwang01

Copy link
Copy Markdown
Collaborator Author

@weiiwang01 Can a screenshot of the Grafana dashboard be included here?

Yes, added in the description, thanks!

@yanksyoon

Copy link
Copy Markdown
Member

[Ninja] @weiiwang01 Hello Weii! Would this item still be relevant?

@weiiwang01

Copy link
Copy Markdown
Collaborator Author

[Ninja] @weiiwang01 Hello Weii! Would this item still be relevant?

Yes, it is waiting to be merged.

@weiiwang01 weiiwang01 closed this Sep 2, 2025
@weiiwang01 weiiwang01 reopened this Sep 2, 2025
Comment thread .github/workflows/test.yaml Outdated
@weiiwang01
weiiwang01 merged commit 08b71e1 into main Sep 5, 2025
35 of 37 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation Improvements or additions to documentation Libraries: Out of sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants