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

Backport OCI Bundle watcher #4430

Closed
ricardomaraschini opened this issue May 16, 2024 · 7 comments
Closed

Backport OCI Bundle watcher #4430

ricardomaraschini opened this issue May 16, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@ricardomaraschini
Copy link
Contributor

Is your feature request related to a problem? Please describe.

On versions v1.28 and v1.29 K0s does not load new OCI bundles without a process restart. This feature has been delivered on v1.30. We currently have clusters running on these "old" versions and we would like to have this back ported if possible.

Describe the solution you would like

The back port involves a small conflict resolution on a file and I have prepared two PRs. I wish they were taken into consideration:

I have tested and the OCI bundle loader works as in v1.30.

Describe alternatives you've considered

On an airgap environment after updating the image bundle we considered restarting k0s because (autopilot only does it when the k0s version changes).

Additional context

Besides the small conflict I have found when back porting this I also had to bump github.com/fsnotify/fsnotify from v1.6.0 to v1.7.0 on the release-1.28 branch (the former version does not support buffered channels).

@ricardomaraschini ricardomaraschini added the enhancement New feature or request label May 16, 2024
@twz123
Copy link
Member

twz123 commented May 16, 2024

Copying over my answer in #4321 over here:

Actually, I'm a pretty conservative guy when it comes to backports. I wouldn't want to port it back so much because it's a pretty substantial change, adding new behavior, not fixing a broken one. Shipping your on k0s builds with the backport applied wouldn't be an option for you in this case? (Just asking 🙃)

@chris-sanders
Copy link

We can carry a forked build, but it's something I try to avoid, especially since version 1.29 still has a long support window. Our preference is always to contribute to the project rather than maintain a fork.

Regarding this being considered a new feature rather than a bug: I see it differently. If we could inform users that "this feature is available starting in 1.30," I would be content. However, my understanding is that an advertised and documented feature (airgap image updates) doesn't work without this change. It seems the intention was for users to use autopilot to update images, but there is no way to do that successfully without manual intervention.

This is precisely why we need to maintain a fork if this change isn't backported. We rely on this documented capability and found that it doesn't fully work, hence the need for the update. There is a possibility that other users, like us, are relying on this feature based on the documentation and would also consider this a bug.

@chris-sanders
Copy link

@jnummelin we can carry a fork or we can help land this, we're getting to a point that we'll need to make a call. Have y'all come to a decision how we should proceed?

@jnummelin
Copy link
Collaborator

@chris-sanders @ricardomaraschini Well, this could be argued both way between a bug and new feature. 😅

After careful consideration and weighing in the risks and benefits, we’ve decided to go ahead with the backports. So those will get into the next patch releases on maintained branches. What tipped the scale is that this is community contribution and it’s very important for us to keep the community happy. Our general stance is still conservative backporting things that are more towards features.

@chris-sanders
Copy link

@jnummelin that's fantastic and we absolutely won't make a habit of back porting. Similarly, if there are any issues with these back port versions related to this change we're happy to look at them as well. Please don't hesitate to ask for help if this does cause an issue down the line.

@twz123
Copy link
Member

twz123 commented May 23, 2024

@chris-sanders A follow-up related to the OCI bundle watcher is #4433. Perhaps you find this important enough to start specifying and implementing this?

@twz123
Copy link
Member

twz123 commented May 27, 2024

The backports are merged.

@twz123 twz123 closed this as completed May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants