search

canonical / istio-operators

Charmed Istio
2 stars 17 forks source link

ci: use latest/candidate for charmcraft channel #387

Closed DnPlas closed 7 months ago

DnPlas commented 7 months ago

This commit pins charmcraft to latest/candidate to avoid issues with the least stable edge channel.

This PR aims at fixing the following error:

charmcraft pack -v
Starting charmcraft, version 2.5.5.post96+g26e3f02                                                                                 
Logging execution to '/home/dplascen/.local/state/charmcraft/log/charmcraft-20240301-124953.845091.log'                            
Bad charmcraft.yaml content:
- ensure this value has at most 78 characters (in field 'summary')                                    
Full execution log: '/home/dplascen/.local/state/charmcraft/log/charmcraft-20240301-124953.845091.log'

Which was caught in the CI of https://github.com/canonical/istio-operators/pull/386.

It would be better to pin this channel than to adapt an older version of this charm. Open to discussions.

ca-scribner commented 7 months ago

To fix this, should we instead shorten our summary field?

iiuc edge adds this restriction. switching to candidate will let CI pass today but sometime in a few weeks when this feature moves to candidate it will just fail again

DnPlas commented 7 months ago

To fix this, should we instead shorten our summary field?

iiuc edge adds this restriction. switching to candidate will let CI pass today but sometime in a few weeks when this feature moves to candidate it will just fail again

Yeah @ca-scribner , that was my first thought, but our charmcraft.yaml does not have a summary field (:

I was also thinking that maybe for track branches we should pin to certain versions of charmcraft. I agree that this change will land in latest/candidate soon, so I was wondering if we should go that route instead. wdty?

ca-scribner commented 7 months ago

oh that is a fun nuance... That sounds like a bug we should probably report then too. I wonder if the summary field, if undefined, gets populated by another field?

I'm +10000 on pinning everything when we put something to a track (microk8s, charmcraft, charm dependencies, ... ). The track is a release, and a release should be a static picture of what we released, not something with unpinned dependencies. It should be possible, but I think we'd have to change some of the actions (to accept revision)

It also might be a pain in the butt to administer if we don't automate the track cutting process. For a single case like this its cool, but for a full bundle release it would be a nightmare

DnPlas commented 7 months ago

oh that is a fun nuance... That sounds like a bug we should probably report then too. I wonder if the summary field, if undefined, gets populated by another field?

Absolutely agree. I'll file an issue (:

EDIT: https://github.com/canonical/charmcraft/issues/1568

I'm +10000 on pinning everything when we put something to a track (microk8s, charmcraft, charm dependencies, ... ). The track is a release, and a release should be a static picture of what we released, not something with unpinned dependencies. It should be possible, but I think we'd have to change some of the actions (to accept revision)

It also might be a pain in the butt to administer if we don't automate the track cutting process. For a single case like this its cool, but for a full bundle release it would be a nightmare

I also think the track branches should have a static version, do you think we can pin a charmcraft version here and then we can have a tracking issue for changing this everywhere?

ca-scribner commented 7 months ago

Is that possible for our CI? I haven't tried, but I think everything accepts channel as an input rather than revision. My expectation is we'd have to add revision as a separate argument (and to the actions like actions-operator and upload-charm)

If there's a way to pin it here though, I'm +1

ca-scribner commented 7 months ago

Added an issue for pinning our build dependencies here, and added it to one of our product feedback planning tasks

DnPlas commented 7 months ago

Is that possible for our CI? I haven't tried, but I think everything accepts channel as an input rather than revision. My expectation is we'd have to add revision as a separate argument (and to the actions like actions-operator and upload-charm)

If there's a way to pin it here though, I'm +1

I don't think at the moment we can accept revisions or versions, I guess what I meant with my previous comment is that we should pin to a channel that was working back when we published this charm.

I was reviewing the channels, though, and found out that the latest/stable version is 2.5.5, which is being worked ATM and may suffer changes, the same changes we are trying to avoid. An earlier version would be 2.2.0, which was last published on Jan 2023, but that is way before the time we published this charm.

For the time being, perhaps we should pin to latest/stable or latest/candidate until https://github.com/canonical/charmcraft/issues/1568 gets fixed? Then I think we could pin to 2.x/stable?

EDIT: I just noticed the suggestion here, I think that would be the best approach for the time being(?). Though we have changed metadata in the past and that has been an issue. I will double check.

DnPlas commented 7 months ago

I have pushed https://github.com/canonical/istio-operators/pull/389, which adds a workaround for the summary issue. Will close this PR.