Skip to content

Conversation

@AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Feb 4, 2024

Revert PR #9713, as it appeared to break the compatibility too much kubernetes/enhancements#3858 (comment)

This reverts commit b2f254f.

Conflicts:
internal/cri/opts/spec_linux_opts.go

Revert PR 9713, as it appeared to break the compatibility too much
kubernetes/enhancements#3858 (comment)

This reverts commit b2f254f.

> Conflicts:
>	internal/cri/opts/spec_linux_opts.go

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@thockin
Copy link
Contributor

thockin commented Feb 4, 2024

I'm asking that we have the discussion about how to handle these sorts of cases.

@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Feb 4, 2024

I'm asking that we have the discussion about how to handle these sorts of cases.

I have spent almost a year to get kubernetes/enhancements#3857 approved
so as to protect mounts without having a breaking change, and I have notified several times that
I would implement the "Plan B" on the CRI runtimes' side in the case the KEP didn't get interests:

Now I have learnt that this was not enough, and realized a necessity to have some explicit guideline on merging potential breaking changes:

@thockin
Copy link
Contributor

thockin commented Feb 5, 2024

Yep. I am not trying to blame you. We have a KEP that did not get the appropriate attention. We have over-burdened SIGs and maintainers. We have multiple projects with their own policies and stances on what constitutes acceptance risk.

Worst, we have a clear lack of data, which makes it impossible to move forward with any real confidence, so we defaulted to the safe position.

This is not a healthy place to be.

Now what to do with it? I think almost everyone will agree that RRO is a better default. But changing it in the kubernetes-specific corner of containers is not the right fix.

@AkihiroSuda
Copy link
Member Author

Now what to do with it? I think almost everyone will agree that RRO is a better default. But changing it in the kubernetes-specific corner of containers is not the right fix.

So, this PR reverts:

I'll wait again to see if there is any chance to get the KEP approved.
If the KEP fails again, this will be probably reimplemented as a Pod annotation without changing the default.

@dims
Copy link
Member

dims commented Feb 5, 2024

cc @henry118 @estesp @mikebrow

@AkihiroSuda AkihiroSuda requested review from mikebrow and mxpv February 7, 2024 11:17
@AkihiroSuda AkihiroSuda added this pull request to the merge queue Feb 8, 2024
Merged via the queue into containerd:main with commit b466b7e Feb 8, 2024
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM .. thx

getting harder to keep track .. made harder for us given we have to support N-different k8s releases and we've had containerd 2.0 working in main over 4-5 releases of k8s

@AkihiroSuda
Copy link
Member Author

#9713 was merged on 2024-02-02 and it survived only for a week, so it doesn't affect existing Kubernetes users.

This is being replaced by:

#9787 doesn't automatically upgrade RO to RRO, and it requires the user to specify recursiveReadOnly mode in the Pod manifest manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cri Container Runtime Interface (CRI) size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants