Skip to content

Conversation

mxpv
Copy link
Member

@mxpv mxpv commented Jul 13, 2022

This forks cri/server -> cri/sbserver package to unblock Sandbox CRI integration. This will allow us to iterate faster and don't worry about affecting existing CRI users.

Because we haven't figured out how exactly we'll be switching between CRI implementations (either an option in the CRI config or runtime or something else), I've added ENABLE_CRI_SANDBOXES env variable, which makes it really easy to enable integration tests.

mxpv added 3 commits July 13, 2022 10:54
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
panic: duplicate metrics collector registration attempted

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
@mxpv mxpv requested a review from mikebrow July 13, 2022 21:45
} else if isLocalHost(host) && u.Scheme == "http" {
// Skipping TLS verification for localhost
transport.TLSClientConfig = &tls.Config{
InsecureSkipVerify: true,

Check failure

Code scanning / CodeQL

Disabled TLS certificate check

InsecureSkipVerify should not be used in production code.
@mxpv
Copy link
Member Author

mxpv commented Jul 13, 2022

/test pull-containerd-node-e2e

@containerd containerd deleted a comment from k8s-ci-robot Jul 13, 2022
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.

a couple comments about the new env variable..

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
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
per slack discussion will remove support for the environment setup as soon as we have some config toml to replace it.. (before r.next)

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

Basically test it in local and it works well. Agree to use config to enable sbserver instead of env.

And we also update some integration cases in follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants