Skip to content

Deprecate legacy shims#4384

Merged
dmcgowan merged 1 commit intocontainerd:masterfrom
AkihiroSuda:deprecate-legacy-shims
Jul 29, 2020
Merged

Deprecate legacy shims#4384
dmcgowan merged 1 commit intocontainerd:masterfrom
AkihiroSuda:deprecate-legacy-shims

Conversation

@AkihiroSuda
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda commented Jul 16, 2020

Fix #4365

@AkihiroSuda AkihiroSuda mentioned this pull request Jul 16, 2020
Closed
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jul 16, 2020

Build succeeded.

@AkihiroSuda AkihiroSuda added this to the 1.4 milestone Jul 16, 2020
fuweid
fuweid approved these changes Jul 16, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

containerd v2.0? nice

thaJeztah
thaJeztah approved these changes Jul 16, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

estesp
estesp approved these changes Jul 16, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83
Copy link
Copy Markdown
Member

cpuguy83 commented Jul 16, 2020

Are we deprecating implementations or API's, or both?
If deprecating an implementation, does that warrant a 2.0 bump?

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jul 16, 2020

@cpuguy83 the PR "marks" it deprecated, but won't remove it until v2.0

@cpuguy83
Copy link
Copy Markdown
Member

cpuguy83 commented Jul 17, 2020

@thaJeztah Right, but this isn't exactly explicit about what is being deprecated, implementation or API, or both.

@AkihiroSuda
Copy link
Copy Markdown
Member Author

AkihiroSuda commented Jul 18, 2020

Are we deprecating implementations or API's, or both?

Both

If deprecating an implementation, does that warrant a 2.0 bump?

I guess yes, but not sure.
@dmcgowan WDYT?

@AkihiroSuda AkihiroSuda force-pushed the deprecate-legacy-shims branch from 32e2213 to 5576f5a Compare July 20, 2020 08:26
@AkihiroSuda
Copy link
Copy Markdown
Member Author

AkihiroSuda commented Jul 20, 2020

Updated the doc to clarify both v1 API and implementation are deprecated

@containerd containerd deleted a comment from theopenlab-ci Bot Jul 20, 2020
dmcgowan
dmcgowan approved these changes Jul 23, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

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

LGTM

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Jul 23, 2020 •
edited
Loading

If deprecating an implementation, does that warrant a 2.0 bump?

I guess yes, but not sure.

I think marking it as 2.0 is the right thing todo. I think to assess whether we could do it before 2.0 we would have to better understand the user impact. We should start logging a warning (if we aren't already) when the client uses the v1 runtime if we are marking it as deprecated in 1.4.

@AkihiroSuda AkihiroSuda force-pushed the deprecate-legacy-shims branch from 5576f5a to 0ca0c60 Compare July 24, 2020 14:34
@AkihiroSuda
Copy link
Copy Markdown
Member Author

AkihiroSuda commented Jul 24, 2020

Added warning prints

@containerd containerd deleted a comment from theopenlab-ci Bot Jul 24, 2020
@AkihiroSuda AkihiroSuda force-pushed the deprecate-legacy-shims branch from 0ca0c60 to e214c8f Compare July 24, 2020 14:54
@containerd containerd deleted a comment from theopenlab-ci Bot Jul 24, 2020
@AkihiroSuda
Copy link
Copy Markdown
Member Author

AkihiroSuda commented Jul 28, 2020

@dmcgowan Still LGTY?

cpuguy83
cpuguy83 approved these changes Jul 28, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

dmcgowan
dmcgowan reviewed Jul 29, 2020
View reviewed changes
Comment thread services/tasks/local.go Outdated
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan Jul 29, 2020

Choose a reason for hiding this comment

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

Mentioning v2.0 in the logs here doesn't feel right to me. This makes it sound like there is active 2.0 planning.

I think with these sort of logs, it is better to be more prescriptive. Say the runtime is deprecated, and what to do about it. Either upgrade to another one, or check out a doc. I think upgrade is the right thing to say here along with the version to upgrade to if possible.

Copy link
Copy Markdown
Member Author

@AkihiroSuda AkihiroSuda Jul 29, 2020

Choose a reason for hiding this comment

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

updated

@AkihiroSuda
Deprecate legacy shims
04b98bb 
Fix containerd#4365

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda AkihiroSuda force-pushed the deprecate-legacy-shims branch from e214c8f to 04b98bb Compare July 29, 2020 16:57
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jul 29, 2020

Build succeeded.

dmcgowan
dmcgowan approved these changes Jul 29, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

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

LGTM

@dmcgowan dmcgowan merged commit d4b1727 into containerd:master Jul 29, 2020
@AkihiroSuda AkihiroSuda mentioned this pull request Aug 4, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan approved these changes

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@estesp estesp estesp approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

@fuweid fuweid fuweid approved these changes

Assignees

No one assigned

Labels

impact/changelog impact/deprecation kind/docs

Projects

None yet

Milestone

1.4

Development

Successfully merging this pull request may close these issues.

Deprecate legacy shims (io.containerd.runtime.v1.linux, io.containerd.runc.v1)

6 participants

@AkihiroSuda @cpuguy83 @thaJeztah @dmcgowan @estesp @fuweid