github
/
k3s
mirror of https://github.com/k3s-io/k3s
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #54597 from sjenning/validate-state-transition 

Automatic merge from submit-queue (batch tested with PRs 54597, 54593, 54081, 54271, 54600). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubelet: check for illegal container state transition

supersedes https://github.com/kubernetes/kubernetes/pull/54530

Puts a state transition check in the kubelet status manager to detect and block illegal transitions; namely from terminated to non-terminated.

@smarterclayton @derekwaynecarr @dashpole @joelsmith @frobware

I confirmed that the reproducer in #54499 does not work with this check in place. The erroneous kubelet status update is rejected:

```
status_manager.go:301] Status update on pod default/test aborted: terminated container test-container attempted illegal transition to non-terminated state
```

After fix https://github.com/kubernetes/kubernetes/pull/54593, I do not see the message with the above mentioned reproducer.
pull/6/head
Kubernetes Submit Queue 2017-10-26 01:44:35 -07:00 committed by GitHub
parent 2ea306a854 449fc027d9
commit 443338b427
1 changed files with 30 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
pkg/kubelet/status/status_manager.go
View File

@ -17,6 +17,7 @@ limitations under the License.
package status
import (
"fmt"
"sort"
"sync"
"time"
@ -264,6 +265,23 @@ func (m *manager) TerminatePod(pod *v1.Pod) {
m.updateStatusInternal(pod, status, true)
}
// checkContainerStateTransition ensures that no container is trying to transition
// from a terminated to non-terminated state, which is illegal and indicates a
// logical error in the kubelet.
func checkContainerStateTransition(oldStatuses, newStatuses []v1.ContainerStatus) error {
for _, newStatus := range newStatuses {
for _, oldStatus := range oldStatuses {
if newStatus.Name != oldStatus.Name {
continue
}
if oldStatus.State.Terminated != nil && newStatus.State.Terminated == nil {
return fmt.Errorf("terminated container %v attempted illegal transition to non-terminated state", newStatus.Name)
}
}
}
return nil
}
// updateStatusInternal updates the internal status cache, and queues an update to the api server if
// necessary. Returns whether an update was triggered.
// This method IS NOT THREAD SAFE and must be called from a locked function.
@ -278,6 +296,18 @@ func (m *manager) updateStatusInternal(pod *v1.Pod, status v1.PodStatus, forceUp
oldStatus = pod.Status
}
// Check for illegal state transition in containers
if pod.Spec.RestartPolicy == v1.RestartPolicyNever {
if err := checkContainerStateTransition(oldStatus.ContainerStatuses, status.ContainerStatuses); err != nil {
glog.Errorf("Status update on pod %v/%v aborted: %v", pod.Namespace, pod.Name, err)
return false
}
if err := checkContainerStateTransition(oldStatus.InitContainerStatuses, status.InitContainerStatuses); err != nil {
glog.Errorf("Status update on pod %v/%v aborted: %v", pod.Namespace, pod.Name, err)
return false
}
}
// Set ReadyCondition.LastTransitionTime.
if _, readyCondition := podutil.GetPodCondition(&status, v1.PodReady); readyCondition != nil {
// Need to set LastTransitionTime.