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

Adjust ContainerStats for Podmod 4.6.0 responses #278

Merged
merged 4 commits into from
Aug 14, 2023

Conversation

sushain97
Copy link
Contributor

Fixes #277

@hashicorp-cla
Copy link

hashicorp-cla commented Aug 7, 2023

CLA assistant check
All committers have signed the CLA.

@Procsiab
Copy link
Contributor

Procsiab commented Aug 7, 2023

I can report that this PR fixes #277 in my environment, on both x86_64 and aarch64 Nomad 1.6.1 clients with Podman 4.6.0-1. Thanks for your quick response @sushain97 💪

@shoenig
Copy link
Member

shoenig commented Aug 7, 2023

@sushain97 check the linter failure

==> Linting source code ...
Error: api/container_stats.go:52:5: shadow: declaration of "err" shadows declaration at line 22 (govet)
	if err := json.Unmarshal(body, &errResponse); err == nil && errResponse.Cause == "container is stopped" {
	   ^
make: *** [GNUmakefile:32: check] Error 1

@sushain97
Copy link
Contributor Author

@shoenig fixed

api/container_stats.go Outdated Show resolved Hide resolved
@sushain97
Copy link
Contributor Author

@lgfa29 does this look good?

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks for the PR @sushain97!

I'm deferring the final merge to @shoenig since he's been working more closely with the Podman driver more recently

@shoenig
Copy link
Member

shoenig commented Aug 14, 2023

@sushain97 I think your fix has a compilation bug:

if _ := json.Unmarshal(body, &errResponse); ...{

needs to be

if _ = json.Unmarshal(body, &errResponse); ... {

@sushain97
Copy link
Contributor Author

@shoenig fixed - the suggestion wasn't quite right.

Would be helpful to have workflows approved for the entire branch/PR :)

Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

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

LGTM; thanks @sushain97 !

@shoenig shoenig merged commit 8bd8d26 into hashicorp:main Aug 14, 2023
@sushain97
Copy link
Contributor Author

No problem! Would it be possible to cut a 0.5.1 release with this fix?

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.

Jobs are not correctly restarted on Podman 4.6.0
5 participants