-
Notifications
You must be signed in to change notification settings - Fork 634
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
reset spec if update returned error #3818
Conversation
@AkihiroSuda could you please review this pr? thanks. |
How to test this? |
1、update memory and gave a wrong args to restart. nerdctl -n k8s.io update --memory=450000000 --restart=123 ef66 2、This will got an error like this: FATA[0000] 1 errors:
unsupported restart policy "123", supported policies are: ["no" "always" "on-failure" "unless-stopped"] 3、 nerdctl -n k8s.io inspect --mode=native <Container ID> inspect we can see the memory is 450000000 now. In fact, the memory is not being updated. |
Can we have an integration test? |
Ok, I will do it. |
dee98b2
to
a99bdc0
Compare
Signed-off-by: zzzzzzzzzy9 <[email protected]>
Done. @AkihiroSuda |
) | ||
|
||
func TestUpdateContainer(t *testing.T) { | ||
testutil.DockerIncompatible(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this depends on inspect --mode=native
to see the memory change, but docker does not support inspect --mode=native
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'm merging this, but wondering if we can omit --mode=native
and run some (docker|nerdctl) exec
command instead
We can't use exec because the update fails and the actual memory is not modified. But the original situation is that even though it has not been modified, |
reset spec if update returned error