From 4e94144d1e7815f52c3a46266df622e8eeaad3d2 Mon Sep 17 00:00:00 2001 From: Sergey Kanzhelev Date: Wed, 20 May 2020 19:01:23 +0000 Subject: [PATCH 1/2] Fix golint failures for kubelet/container --- pkg/kubelet/container/container_gc.go | 6 +-- pkg/kubelet/container/helpers.go | 10 +++-- pkg/kubelet/container/ref.go | 5 ++- pkg/kubelet/container/resize.go | 2 +- pkg/kubelet/container/runtime.go | 47 +++++++++++++++------ pkg/kubelet/container/runtime_cache.go | 1 + pkg/kubelet/container/runtime_cache_fake.go | 5 ++- pkg/kubelet/container/sync_result.go | 41 ++++++++++++------ 8 files changed, 83 insertions(+), 34 deletions(-) diff --git a/pkg/kubelet/container/container_gc.go b/pkg/kubelet/container/container_gc.go index 909545bea4b..c9a6197ea95 100644 --- a/pkg/kubelet/container/container_gc.go +++ b/pkg/kubelet/container/container_gc.go @@ -23,7 +23,7 @@ import ( "k8s.io/klog/v2" ) -// Specified a policy for garbage collecting containers. +// ContainerGCPolicy specifies a policy for garbage collecting containers. type ContainerGCPolicy struct { // Minimum age at which a container can be garbage collected, zero for no limit. MinAge time.Duration @@ -36,7 +36,7 @@ type ContainerGCPolicy struct { MaxContainers int } -// Manages garbage collection of dead containers. +// ContainerGC manages garbage collection of dead containers. // // Implementation is thread-compatible. type ContainerGC interface { @@ -64,7 +64,7 @@ type realContainerGC struct { sourcesReadyProvider SourcesReadyProvider } -// New ContainerGC instance with the specified policy. +// NewContainerGC creates a new instance of ContainerGC with the specified policy. func NewContainerGC(runtime Runtime, policy ContainerGCPolicy, sourcesReadyProvider SourcesReadyProvider) (ContainerGC, error) { if policy.MinAge < 0 { return nil, fmt.Errorf("invalid minimum garbage collection age: %v", policy.MinAge) diff --git a/pkg/kubelet/container/helpers.go b/pkg/kubelet/container/helpers.go index 8846dfda5d1..bb14bc560d6 100644 --- a/pkg/kubelet/container/helpers.go +++ b/pkg/kubelet/container/helpers.go @@ -102,8 +102,8 @@ func HashContainer(container *v1.Container) uint64 { hash := fnv.New32a() // Omit nil or empty field when calculating hash value // Please see https://github.com/kubernetes/kubernetes/issues/53644 - containerJson, _ := json.Marshal(container) - hashutil.DeepHashObject(hash, containerJson) + containerJSON, _ := json.Marshal(container) + hashutil.DeepHashObject(hash, containerJSON) return uint64(hash.Sum32()) } @@ -141,6 +141,7 @@ func ExpandContainerCommandOnlyStatic(containerCommand []string, envs []v1.EnvVa return command } +// ExpandContainerVolumeMounts expands the subpath of the given VolumeMount by replacing variable references `with the values of given EnvVar. func ExpandContainerVolumeMounts(mount v1.VolumeMount, envs []EnvVar) (string, error) { envmap := EnvVarsToMap(envs) @@ -159,6 +160,7 @@ func ExpandContainerVolumeMounts(mount v1.VolumeMount, envs []EnvVar) (string, e return expanded, nil } +// ExpandContainerCommandAndArgs expands the given Container's command by replacing variable references `with the values of given EnvVar. func ExpandContainerCommandAndArgs(container *v1.Container, envs []EnvVar) (command []string, args []string) { mapping := expansion.MappingFuncFor(EnvVarsToMap(envs)) @@ -177,7 +179,7 @@ func ExpandContainerCommandAndArgs(container *v1.Container, envs []EnvVar) (comm return command, args } -// Create an event recorder to record object's event except implicitly required container's, like infra container. +// FilterEventRecorder creates an event recorder to record object's event except implicitly required container's, like infra container. func FilterEventRecorder(recorder record.EventRecorder) record.EventRecorder { return &innerEventRecorder{ recorder: recorder, @@ -220,11 +222,13 @@ func (irecorder *innerEventRecorder) AnnotatedEventf(object runtime.Object, anno } +// IsHostNetworkPod returns whether the host networking requested for the given Pod. // Pod must not be nil. func IsHostNetworkPod(pod *v1.Pod) bool { return pod.Spec.HostNetwork } +// ConvertPodStatusToRunningPod returns Pod given PodStatus and container runtime string. // TODO(random-liu): Convert PodStatus to running Pod, should be deprecated soon func ConvertPodStatusToRunningPod(runtimeName string, podStatus *PodStatus) Pod { runningPod := Pod{ diff --git a/pkg/kubelet/container/ref.go b/pkg/kubelet/container/ref.go index 7e3f188bc88..3d366dc188c 100644 --- a/pkg/kubelet/container/ref.go +++ b/pkg/kubelet/container/ref.go @@ -19,14 +19,15 @@ package container import ( "fmt" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" utilfeature "k8s.io/apiserver/pkg/util/feature" ref "k8s.io/client-go/tools/reference" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/features" ) -var ImplicitContainerPrefix string = "implicitly required container " +// ImplicitContainerPrefix is a container name prefix that will indicate that container was started implicitly (like the pod infra container). +var ImplicitContainerPrefix = "implicitly required container " // GenerateContainerRef returns an *v1.ObjectReference which references the given container // within the given pod. Returns an error if the reference can't be constructed or the diff --git a/pkg/kubelet/container/resize.go b/pkg/kubelet/container/resize.go index 34e40a95b46..921c43564b4 100644 --- a/pkg/kubelet/container/resize.go +++ b/pkg/kubelet/container/resize.go @@ -21,7 +21,7 @@ import ( "k8s.io/client-go/tools/remotecommand" ) -// handleResizing spawns a goroutine that processes the resize channel, calling resizeFunc for each +// HandleResizing spawns a goroutine that processes the resize channel, calling resizeFunc for each // remotecommand.TerminalSize received from the channel. The resize channel must be closed elsewhere to stop the // goroutine. func HandleResizing(resize <-chan remotecommand.TerminalSize, resizeFunc func(size remotecommand.TerminalSize)) { diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index 8a4db8970fb..dda480e2edc 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -34,6 +34,7 @@ import ( "k8s.io/kubernetes/pkg/volume" ) +// Version interface allow to consume the runtime versions - compare and format to string. type Version interface { // Compare compares two versions of the runtime. On success it returns -1 // if the version is less than the other, 1 if it is greater than the other, @@ -87,7 +88,7 @@ type Runtime interface { GetPods(all bool) ([]*Pod, error) // GarbageCollect removes dead containers using the specified container gc policy // If allSourcesReady is not true, it means that kubelet doesn't have the - // complete list of pods from all avialble sources (e.g., apiserver, http, + // complete list of pods from all available sources (e.g., apiserver, http, // file). In this case, garbage collector should refrain itself from aggressive // behavior such as removing all containers of unrecognized pods (yet). // If evictNonDeletedPods is set to true, containers and sandboxes belonging to pods @@ -130,6 +131,7 @@ type StreamingRuntime interface { GetPortForward(podName, podNamespace string, podUID types.UID, ports []int32) (*url.URL, error) } +// ImageService interfaces allows to work with image service. type ImageService interface { // PullImage pulls an image from the network to local storage using the supplied // secrets if necessary. It returns a reference (digest or ID) to the pulled image. @@ -145,10 +147,12 @@ type ImageService interface { ImageStats() (*ImageStats, error) } +// ContainerAttacher interface allows to attach a container. type ContainerAttacher interface { AttachContainer(id ContainerID, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool, resize <-chan remotecommand.TerminalSize) (err error) } +// ContainerCommandRunner interface allows to run command in a container. type ContainerCommandRunner interface { // RunInContainer synchronously executes the command in the container, and returns the output. // If the command completes with a non-0 exit code, a k8s.io/utils/exec.ExitError will be returned. @@ -167,7 +171,7 @@ type Pod struct { // running containers, or mixed with dead ones (when GetPods(true)). Containers []*Container // List of sandboxes associated with this pod. The sandboxes are converted - // to Container temporariliy to avoid substantial changes to other + // to Container temporarily to avoid substantial changes to other // components. This is only populated by kuberuntime. // TODO: use the runtimeApi.PodSandbox type directly. Sandboxes []*Container @@ -191,11 +195,12 @@ type ContainerID struct { ID string } +// BuildContainerID returns the ContainerID given type and id. func BuildContainerID(typ, ID string) ContainerID { return ContainerID{Type: typ, ID: ID} } -// Convenience method for creating a ContainerID from an ID string. +// ParseContainerID is a convenience method for creating a ContainerID from an ID string. func ParseContainerID(containerID string) ContainerID { var id ContainerID if err := id.ParseString(containerID); err != nil { @@ -204,6 +209,7 @@ func ParseContainerID(containerID string) ContainerID { return id } +// ParseString converts given string into ContainerID func (c *ContainerID) ParseString(data string) error { // Trim the quotes and split the type and ID. parts := strings.Split(strings.Trim(data, "\""), "://") @@ -218,14 +224,17 @@ func (c *ContainerID) String() string { return fmt.Sprintf("%s://%s", c.Type, c.ID) } +// IsEmpty returns whether given ContainerID is empty. func (c *ContainerID) IsEmpty() bool { return *c == ContainerID{} } +// MarshalJSON formats a given ContainerID into a byte array. func (c *ContainerID) MarshalJSON() ([]byte, error) { return []byte(fmt.Sprintf("%q", c.String())), nil } +// UnmarshalJSON parses ContainerID from a given array of bytes. func (c *ContainerID) UnmarshalJSON(data []byte) error { return c.ParseString(string(data)) } @@ -233,6 +242,7 @@ func (c *ContainerID) UnmarshalJSON(data []byte) error { // DockerID is an ID of docker container. It is a type to make it clear when we're working with docker container Ids type DockerID string +// ContainerID converts DockerID into a ContainerID. func (id DockerID) ContainerID() ContainerID { return ContainerID{ Type: "docker", @@ -240,13 +250,17 @@ func (id DockerID) ContainerID() ContainerID { } } +// ContainerState represents the state of a container type ContainerState string const ( + // ContainerStateCreated indicates a container that has been created (e.g. with docker create) but not started. ContainerStateCreated ContainerState = "created" + // ContainerStateRunning indicates a currently running container. ContainerStateRunning ContainerState = "running" - ContainerStateExited ContainerState = "exited" - // This unknown encompasses all the states that we currently don't care. + // ContainerStateExited indicates a container that ran and completed ("stopped" in other contexts, although a created container is technically also "stopped"). + ContainerStateExited ContainerState = "exited" + // ContainerStateUnknown encompasses all the states that we currently don't care (like restarting, paused, dead). ContainerStateUnknown ContainerState = "unknown" ) @@ -331,7 +345,7 @@ func (podStatus *PodStatus) FindContainerStatusByName(containerName string) *Con return nil } -// Get container status of all the running containers in a pod +// GetRunningContainerStatuses returns container status of all the running containers in a pod func (podStatus *PodStatus) GetRunningContainerStatuses() []*ContainerStatus { runningContainerStatuses := []*ContainerStatus{} for _, containerStatus := range podStatus.ContainerStatuses { @@ -342,7 +356,7 @@ func (podStatus *PodStatus) GetRunningContainerStatuses() []*ContainerStatus { return runningContainerStatuses } -// Basic information about a container image. +// Image contains basic information about a container image. type Image struct { // ID of the image. ID string @@ -356,16 +370,19 @@ type Image struct { Spec ImageSpec } +// EnvVar represents the environment variable. type EnvVar struct { Name string Value string } +// Annotation represents an annotation. type Annotation struct { Name string Value string } +// Mount represents a volume mount. type Mount struct { // Name of the volume mount. // TODO(yifan): Remove this field, as this is not representing the unique name of the mount, @@ -383,6 +400,7 @@ type Mount struct { Propagation runtimeapi.MountPropagation } +// PortMapping contains information about the port mapping. type PortMapping struct { // Name of the port mapping Name string @@ -396,6 +414,7 @@ type PortMapping struct { HostIP string } +// DeviceInfo contains information about the device. type DeviceInfo struct { // Path on host for mapping PathOnHost string @@ -452,6 +471,7 @@ type VolumeInfo struct { InnerVolumeSpecName string } +// VolumeMap represents the map of volumes. type VolumeMap map[string]VolumeInfo // RuntimeConditionType is the types of required runtime conditions. @@ -482,9 +502,9 @@ func (r *RuntimeStatus) GetRuntimeCondition(t RuntimeConditionType) *RuntimeCond } // String formats the runtime status into human readable string. -func (s *RuntimeStatus) String() string { +func (r *RuntimeStatus) String() string { var ss []string - for _, c := range s.Conditions { + for _, c := range r.Conditions { ss = append(ss, c.String()) } return fmt.Sprintf("Runtime Conditions: %s", strings.Join(ss, ", ")) @@ -507,6 +527,7 @@ func (c *RuntimeCondition) String() string { return fmt.Sprintf("%s=%t reason:%s message:%s", c.Type, c.Status, c.Reason, c.Message) } +// Pods represents the list of pods type Pods []*Pod // FindPodByID finds and returns a pod in the pod list by UID. It will return an empty pod @@ -553,6 +574,7 @@ func (p *Pod) FindContainerByName(containerName string) *Container { return nil } +// FindContainerByID returns a container in the pod with the given ContainerID. func (p *Pod) FindContainerByID(id ContainerID) *Container { for _, c := range p.Containers { if c.ID == id { @@ -562,6 +584,7 @@ func (p *Pod) FindContainerByID(id ContainerID) *Container { return nil } +// FindSandboxByID returns a sandbox in the pod with the given ContainerID. func (p *Pod) FindSandboxByID(id ContainerID) *Container { for _, c := range p.Sandboxes { if c.ID == id { @@ -600,12 +623,12 @@ func GetPodFullName(pod *v1.Pod) string { return pod.Name + "_" + pod.Namespace } -// Build the pod full name from pod name and namespace. +// BuildPodFullName builds the pod full name from pod name and namespace. func BuildPodFullName(name, namespace string) string { return name + "_" + namespace } -// Parse the pod full name. +// ParsePodFullName parsed the pod full name. func ParsePodFullName(podFullName string) (string, string, error) { parts := strings.Split(podFullName, "_") if len(parts) != 2 || parts[0] == "" || parts[1] == "" { @@ -618,7 +641,7 @@ func ParsePodFullName(podFullName string) (string, string, error) { // completely optional settings. type Option func(Runtime) -// Sort the container statuses by creation time. +// SortContainerStatusesByCreationTime sorts the container statuses by creation time. type SortContainerStatusesByCreationTime []*ContainerStatus func (s SortContainerStatusesByCreationTime) Len() int { return len(s) } diff --git a/pkg/kubelet/container/runtime_cache.go b/pkg/kubelet/container/runtime_cache.go index d15852f886a..0d7a1d399c8 100644 --- a/pkg/kubelet/container/runtime_cache.go +++ b/pkg/kubelet/container/runtime_cache.go @@ -26,6 +26,7 @@ var ( defaultCachePeriod = time.Second * 2 ) +// RuntimeCache is in interface for obtaining cached Pods. type RuntimeCache interface { GetPods() ([]*Pod, error) ForceUpdateIfOlder(time.Time) error diff --git a/pkg/kubelet/container/runtime_cache_fake.go b/pkg/kubelet/container/runtime_cache_fake.go index 59a6288d536..0c07c7edfd2 100644 --- a/pkg/kubelet/container/runtime_cache_fake.go +++ b/pkg/kubelet/container/runtime_cache_fake.go @@ -16,7 +16,7 @@ limitations under the License. package container -// TestRunTimeCache embeds runtimeCache with some additional methods for testing. +// TestRuntimeCache embeds runtimeCache with some additional methods for testing. // It must be declared in the container package to have visibility to runtimeCache. // It cannot be in a "..._test.go" file in order for runtime_cache_test.go to have cross-package visibility to it. // (cross-package declarations in test files cannot be used from dot imports if this package is vendored) @@ -24,18 +24,21 @@ type TestRuntimeCache struct { runtimeCache } +// UpdateCacheWithLock updates the cache with the lock. func (r *TestRuntimeCache) UpdateCacheWithLock() error { r.Lock() defer r.Unlock() return r.updateCache() } +// GetCachedPods returns the cached pods. func (r *TestRuntimeCache) GetCachedPods() []*Pod { r.Lock() defer r.Unlock() return r.pods } +// NewTestRuntimeCache creates a new instance of TestRuntimeCache. func NewTestRuntimeCache(getter podsGetter) *TestRuntimeCache { return &TestRuntimeCache{ runtimeCache: runtimeCache{ diff --git a/pkg/kubelet/container/sync_result.go b/pkg/kubelet/container/sync_result.go index ef1b2a97d5e..58c8de8ba1c 100644 --- a/pkg/kubelet/container/sync_result.go +++ b/pkg/kubelet/container/sync_result.go @@ -25,7 +25,7 @@ import ( // TODO(random-liu): We need to better organize runtime errors for introspection. -// Container Terminated and Kubelet is backing off the restart +// ErrCrashLoopBackOff returned when a container Terminated and Kubelet is backing off the restart. var ErrCrashLoopBackOff = errors.New("CrashLoopBackOff") var ( @@ -35,17 +35,26 @@ var ( ) var ( - ErrRunContainer = errors.New("RunContainerError") - ErrKillContainer = errors.New("KillContainerError") - ErrVerifyNonRoot = errors.New("VerifyNonRootError") + // ErrRunContainer returned when runtime failed to start any of pod's container. + ErrRunContainer = errors.New("RunContainerError") + // ErrKillContainer returned when runtime failed to kill any of pod's containers. + ErrKillContainer = errors.New("KillContainerError") + // ErrVerifyNonRoot returned if the container or image will run as the root user. + ErrVerifyNonRoot = errors.New("VerifyNonRootError") + // ErrRunInitContainer returned when container init failed. ErrRunInitContainer = errors.New("RunInitContainerError") + // ErrCreatePodSandbox returned when runtime failed to create a sandbox for pod. ErrCreatePodSandbox = errors.New("CreatePodSandboxError") + // ErrConfigPodSandbox returned when runetime failed to get pod sandbox config from pod. ErrConfigPodSandbox = errors.New("ConfigPodSandboxError") - ErrKillPodSandbox = errors.New("KillPodSandboxError") + // ErrKillPodSandbox returned when runtime failed to stop pod's sandbox. + ErrKillPodSandbox = errors.New("KillPodSandboxError") ) var ( - ErrSetupNetwork = errors.New("SetupNetworkError") + // ErrSetupNetwork returned when network setup failed. + ErrSetupNetwork = errors.New("SetupNetworkError") + // ErrTeardownNetwork returned when network tear down failed. ErrTeardownNetwork = errors.New("TeardownNetworkError") ) @@ -54,14 +63,22 @@ var ( type SyncAction string const ( - StartContainer SyncAction = "StartContainer" - KillContainer SyncAction = "KillContainer" - SetupNetwork SyncAction = "SetupNetwork" - TeardownNetwork SyncAction = "TeardownNetwork" - InitContainer SyncAction = "InitContainer" + // StartContainer action + StartContainer SyncAction = "StartContainer" + // KillContainer action + KillContainer SyncAction = "KillContainer" + // SetupNetwork action + SetupNetwork SyncAction = "SetupNetwork" + // TeardownNetwork action + TeardownNetwork SyncAction = "TeardownNetwork" + // InitContainer action + InitContainer SyncAction = "InitContainer" + // CreatePodSandbox action CreatePodSandbox SyncAction = "CreatePodSandbox" + // ConfigPodSandbox action ConfigPodSandbox SyncAction = "ConfigPodSandbox" - KillPodSandbox SyncAction = "KillPodSandbox" + // KillPodSandbox action + KillPodSandbox SyncAction = "KillPodSandbox" ) // SyncResult is the result of sync action. From a4bfc732f7fdb60429cb247f3c7f45d7b0941854 Mon Sep 17 00:00:00 2001 From: Sergey Kanzhelev Date: Thu, 21 May 2020 04:15:44 +0000 Subject: [PATCH 2/2] minor correction in comments --- pkg/kubelet/container/helpers.go | 2 +- pkg/kubelet/container/runtime.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/kubelet/container/helpers.go b/pkg/kubelet/container/helpers.go index bb14bc560d6..dcdc8979d81 100644 --- a/pkg/kubelet/container/helpers.go +++ b/pkg/kubelet/container/helpers.go @@ -141,7 +141,7 @@ func ExpandContainerCommandOnlyStatic(containerCommand []string, envs []v1.EnvVa return command } -// ExpandContainerVolumeMounts expands the subpath of the given VolumeMount by replacing variable references `with the values of given EnvVar. +// ExpandContainerVolumeMounts expands the subpath of the given VolumeMount by replacing variable references with the values of given EnvVar. func ExpandContainerVolumeMounts(mount v1.VolumeMount, envs []EnvVar) (string, error) { envmap := EnvVarsToMap(envs) diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index dda480e2edc..3658e57d415 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -260,7 +260,7 @@ const ( ContainerStateRunning ContainerState = "running" // ContainerStateExited indicates a container that ran and completed ("stopped" in other contexts, although a created container is technically also "stopped"). ContainerStateExited ContainerState = "exited" - // ContainerStateUnknown encompasses all the states that we currently don't care (like restarting, paused, dead). + // ContainerStateUnknown encompasses all the states that we currently don't care about (like restarting, paused, dead). ContainerStateUnknown ContainerState = "unknown" )