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

Add tox env for noble #174

Merged
merged 5 commits into from
Aug 31, 2024
Merged

Add tox env for noble #174

merged 5 commits into from
Aug 31, 2024

Conversation

sed-i
Copy link
Contributor

@sed-i sed-i commented Aug 27, 2024

Issue

We need to release a noble (24.04) revision, which only charmcraft v3 can do: with charmcraft 2.x, we'd get for noble:

No suitable 'build-on' environment found in 'bases[0]' configuration.

With charmcraft 3, if we try to add it to the existing list of build-on, charmcraft errors out with:

Bad charmcraft.yaml content:
- base requires 'platforms' definition: {'name': 'ubuntu', 'channel': '24.04', 'architectures': ['arm64']} (in field 'bases[0]')

because charmcraft v3 won't pack noble when charmcraft.yaml has the old schema.

Solution

  • Use charmcraft 3 for all builds, but have two different charmcraft.yaml files:
    • charmcraft-22.04.yaml, which is identical to before;
    • charmcraft-24.04.yaml, with the new base schema.
  • Also keep charmcraft.yaml so existing github CI still works as before.
  • Keep releasing noble manually until a better solution emerges.

Fixes #155.

Drive-by fixes:

  • Consolidate metadata, config into charmcraft.yaml.

Context

Testing Instructions

Upgrade Notes

tox.ini Outdated Show resolved Hide resolved
@sed-i sed-i marked this pull request as ready for review August 30, 2024 03:58
@sed-i sed-i requested a review from a team as a code owner August 30, 2024 03:58
tox.ini Outdated
Comment on lines 115 to 117
# 22.04 is the default so we don't need to restore anything after
22.04: sh -c "ln -srf {toxinidir}/charmcraft-22.04.yaml {toxinidir}/charmcraft.yaml && charmcraft pack --project-dir={toxinidir}"
24.04: sh -c "ln -srf {toxinidir}/charmcraft-24.04.yaml {toxinidir}/charmcraft.yaml && charmcraft pack --project-dir={toxinidir}; ln -srf {toxinidir}/charmcraft-22.04.yaml {toxinidir}/charmcraft.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

What about making these do:

  • backup current charmcraft.yaml
  • make their own charmcraft.yaml pointing to the right thing
  • pack
  • restore charmcraft.yaml
    ?

It feels odd that 22.04 and 24.04 have different properties around their statefulness. Agreed that it'll work atm, but this is an easy thing for someone to misuse without realizing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what's the benefit of this approach.

  • If we symlink then if someone interrupts the 24.04 and git add . then we'd see a small diff (the symlink changed from 22.04 to 24.04).
  • If we use a third copy of the yaml + untracked backup, then if someone git add . then we'd see a slightly larger diff (the charmcraft.yaml will have an entire section replaced).

A symlink seems cleaner. What am I missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not against a symlink, just the asymmetry between the two versions:

  • 22.04 changes the charmcraft.yaml to the current case, packs
  • 24.04 changes the charmcraft.yaml to the current case, packs, and then back to a different case

They don't feel equivalent enough to bundle together. They'll work atm, but someone in future is going to miss that subtlety. What I'm proposing is:

  • X: backs up charmcraft.yaml, changes charmcraft.yaml to X, packs, restores backed up charmcraft.yaml

That way all versions of the environment work the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative would be have only a pack-24.04 environment (no 22.04). That feels more suitable imo, since the idea is packing for 24.04 is currently a special, manual case.

Copy link
Contributor

Choose a reason for hiding this comment

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

hah, I see @lucabello asked about this too. I like his original suggestion (or, doing the same thing but with backing up symlinks).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huh, yeah, might as well drop the 22.04 option. Thanks!

Copy link
Contributor

@ca-scribner ca-scribner left a comment

Choose a reason for hiding this comment

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

lgtm, but I'd rework the tox env to either work the same for all bases or to just implement a single pack-24.04 env

@sed-i sed-i merged commit 8caf3ec into main Aug 31, 2024
13 checks passed
@sed-i sed-i deleted the feature/noble branch August 31, 2024 03:18
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.

grafana-agent charm based on Noble
3 participants