canonical / solutions-engineering-automation

Repo for automating tasks for Solutions Engineering Team.
0 stars 4 forks source link

[Terraform] Update `charmcraft` channel in workflow template #45

Closed chanchiwai-ray closed 2 weeks ago

chanchiwai-ray commented 4 weeks ago

The charmcraft channel is currently pinned to 2.x/stable to avoid breaking changes to our workflow since our charmcraft.yaml are not compatible with the latest charmcraft (3/stable). We should update our charm's charmcraft.yaml to make it compatible with charmcraft 3/stable.

We should remove the charmcraft pin in terraform automation: e.g. https://github.com/canonical/solutions-engineering-automation/blob/main/terraform-plans/templates/github/charm_check.yaml.tftpl#L58

samuelallan72 commented 2 weeks ago

This is offtopic in this repository; I'll open bugs on our charm repos to track upgrading to charmcraft 3 for our charms.

chanchiwai-ray commented 2 weeks ago

No, this is not off topic: see https://github.com/canonical/solutions-engineering-automation/blob/main/terraform-plans/templates/github/charm_check.yaml.tftpl#L58 for example.

samuelallan72 commented 2 weeks ago

@chanchiwai-ray but we can't update it here in one go. For each relevent charm repository, we need to update charmcraft.yaml and test the workflows there, then port it to this automation repo for that single charm. So this issue is not in the scope of issues here.

chanchiwai-ray commented 2 weeks ago

The issue is to keep track of the "temporary" pin in this automation repository. Or is it not temporary? Do we want to keep pin a specific version for all our tool chains for building or testing a charm (e.g. juju version, charmcraft version, lxd version etc?).

You can still update charmcraft.yaml and test the workflow in each repo (and merge them), then port it here when all repos are tested. (in the meantime, you can't make any change to the terraform repo because it will override the change you make for charmcraft 3)

samuelallan72 commented 2 weeks ago

then port it here when all repo is tested.

No we can actually do it incrementally, by introducing a variable in the template (eg. charmcraft_channel), like what we do for the architecture (runs_on variable). So we'll never have a situation where we need to port all charms then update automation; we update automation incrementally as we update the charms.

Or is not temporary? Do we want to keep pin a specific version for all our tool chains for building or testing a charm

Yes this is not temporary. Or at least, it's temporary only in the sense that eventually we want to upgrade the charms to support the next version and update the pin accordingly. The charmcraft 2 pinning wasn't a temporary fix, it was declaring which version of charmcraft our charms build with.

chanchiwai-ray commented 2 weeks ago

No we can actually do it incrementally

Yes, we should do that if the pin is not temporary, It looked to me that's only a temporary hardcoded value. And we will remove afterward. Please make it a variable and close this issue with that PR

chanchiwai-ray commented 2 weeks ago

Yes this is not temporary. Or at least, it's temporary only in the sense that eventually we want to upgrade the charms to support

We don't pin those tool chains, usually. We just pick the latest one, and help the team and organization to fix the issue, or report the issue if we found one. If the policy changes here, I am also okay with that.

samuelallan72 commented 2 weeks ago

We don't pin those tool chains, usually.

And that's how we keep running into issues where everything breaks all at once like the charmcraft 2 -> 3, tox 3 -> 4, etc... :/ I don't know if we have an official policy, but we should be at least pinning major versions where possible to avoid an upstream update from breaking everything on our side.

samuelallan72 commented 2 weeks ago

Please make it a variable and close this issue with that PR

We don't need a variable yet, because we don't have any charms with the workflows in automation that support charmcraft 3 yet. We can refactor into a variable when required. :)

chanchiwai-ray commented 2 weeks ago

We don't need a variable yet, because we don't have any charms with the workflows in automation that support charmcraft 3 yet. We can refactor into a variable when required. :)

Thanks, that sounds good to me. Just remember to close this when you do the refactoring :)

samuelallan72 commented 2 weeks ago

We can close it now, because this isn't an issue with this repository. :)