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.

Sign up for GitHub

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

Jump to bottom

Fixes invalid JSON in crictl info #1486

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Fixes invalid JSON in crictl info #1486

merged 1 commit into from
Jul 11, 2024

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Jul 4, 2024 •
edited
Loading

What type of PR is this?

/kind bug

What this PR does / why we need it:

containerd on Windows may not escape the return message which may result in invalid JSON in crictl info.

Message from containerd:
cni config load failed: no network config found in C:\Program Files \containerd\cni\conf: cni plugin

not initialized: failed to load cni config

Cherry-picked: 88df400

Which issue(s) this PR fixes:

Fixes #1426

Special notes for your reviewer:

Taking over the reverts of #1429 and #1446 to finally fix that issue.

Does this PR introduce a user-facing change?

Fixed invalid JSON in crictl info on Windows.

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 4, 2024
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 4, 2024
@saschagrunert saschagrunert changed the title Fixes invalid JSON in crictl info WIP: Fixes invalid JSON in crictl info Jul 4, 2024
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 4, 2024
@saschagrunert saschagrunert changed the title WIP: Fixes invalid JSON in crictl info Fixes invalid JSON in crictl info Jul 4, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 4, 2024
kwilczynski
kwilczynski reviewed Jul 8, 2024
View reviewed changes
cmd/crictl/util_test.go Outdated Show resolved Hide resolved
kwilczynski
kwilczynski reviewed Jul 8, 2024
View reviewed changes
cmd/crictl/util_test.go Outdated Show resolved Hide resolved
@kwilczynski
Copy link
Member

Thank you for fixing this! Much appreciated!

@kwilczynski
Copy link
Member

/approve
/lgtm

@k8s-ci-robot
Copy link
Contributor

@kwilczynski: changing LGTM is restricted to collaborators

In response to this:

/approve
/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

kwilczynski
kwilczynski reviewed Jul 8, 2024
View reviewed changes
cmd/crictl/util.go Outdated Show resolved Hide resolved
@laozc @saschagrunert
Fixes invalid JSON in crictl info
0d18e2d 
containerd on Windows may not escape the return message which may
result in invalid JSON in crictl info.

Message from containerd:
cni config load failed: no network config found in C:\Program Files
\containerd\cni\conf: cni plugin

not initialized: failed to load cni config

Cherry-picked: 88df400
Signed-off-by: Sascha Grunert <[email protected]>
@saschagrunert
Copy link
Member Author

@kubernetes-sigs/cri-tools-maintainers PTAL

@saschagrunert saschagrunert mentioned this pull request Jul 10, 2024
Closed
@kwilczynski
Copy link
Member

Still looks good!

/approve
/lgtm

@k8s-ci-robot
Copy link
Contributor

@kwilczynski: changing LGTM is restricted to collaborators

In response to this:

Still looks good!

/approve
/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@saschagrunert saschagrunert mentioned this pull request Jul 10, 2024
Merged
@saschagrunert
Copy link
Member Author

@kubernetes-sigs/cri-tools-maintainers PTAL

@kwilczynski
Copy link
Member

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kwilczynski, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit a9d9845 into kubernetes-sigs:master Jul 11, 2024
23 checks passed
@saschagrunert saschagrunert deleted the invalid-json branch July 11, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kwilczynski kwilczynski kwilczynski left review comments

@feiskyer feiskyer Awaiting requested review from feiskyer

@klueska klueska Awaiting requested review from klueska

Assignees

@kwilczynski kwilczynski

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

kubeadm join fails on Windows return getting status of runtime: invalid character 'P' in string escape code
4 participants
@saschagrunert @kwilczynski @k8s-ci-robot @laozc