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

ci: use latest/candidate for charmcraft channel #387

Closed
wants to merge 1 commit into from

Conversation

DnPlas
Copy link
Contributor

@DnPlas DnPlas commented Mar 1, 2024

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 #386.

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

This commit pins charmcraft to latest/candidate to avoid issues with the least
stable edge channel.
@ca-scribner
Copy link
Contributor

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
Copy link
Contributor Author

DnPlas commented Mar 1, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

DnPlas commented Mar 1, 2024

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: canonical/charmcraft#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
Copy link
Contributor

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
Copy link
Contributor

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

@DnPlas
Copy link
Contributor Author

DnPlas commented Mar 4, 2024

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 canonical/charmcraft#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
Copy link
Contributor Author

DnPlas commented Mar 4, 2024

I have pushed #389, which adds a workaround for the summary issue. Will close this PR.

@DnPlas DnPlas closed this Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants