Improve documentation of PodController and relating interfaces (#2298)

* Extract PodControllerProcessor interface and default implementation * Extract PodCommandExecutorProcessor interface and default implementation * Extract PodFileWriterProcessor interface and default implementation * Apply suggestions from code review - copyright header Co-authored-by: Pavan Navarathna <[email protected]> * Apply suggestions from code review - metav1 Co-authored-by: Pavan Navarathna <[email protected]> * Improve documentation of PodController and relating interfaces * Apply suggestions from code review Co-authored-by: Pavan Navarathna <[email protected]> --------- Co-authored-by: Pavan Navarathna <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
kanisterio · Sep 14, 2023 · d847e11 · d847e11
1 parent e3fadce
commit d847e11
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 10 deletions.
diff --git a/pkg/kube/pod_command_executor.go b/pkg/kube/pod_command_executor.go
@@ -21,6 +21,9 @@ import (
 	"k8s.io/client-go/kubernetes"
 )
 
+// ExecError is an error returned by PodCommandExecutor.Exec
+// It contains not only error happened during an execution, but also keeps tails of stdout/stderr streams.
+// These tails could be used by the invoker to construct more precise error.
 type ExecError struct {
 	error
 	stdout LogTail
@@ -48,7 +51,8 @@ func (e *ExecError) Stderr() string {
 	return e.stderr.ToString()
 }
 
-// PodCommandExecutor allows us to execute command within the pod
+// PodCommandExecutor provides a way to execute a command within the pod.
+// Is intended to be returned by PodController and works with pod controlled by it.
 type PodCommandExecutor interface {
 	Exec(ctx context.Context, command []string, stdin io.Reader, stdout, stderr io.Writer) error
 }
@@ -64,6 +68,7 @@ type podCommandExecutor struct {
 }
 
 // Exec runs the command and logs stdout and stderr.
+// In case of execution error, ExecError will be returned
 func (p *podCommandExecutor) Exec(ctx context.Context, command []string, stdin io.Reader, stdout, stderr io.Writer) error {
 	var (
 		stderrTail = NewLogTail(logTailDefaultLength)

diff --git a/pkg/kube/pod_controller.go b/pkg/kube/pod_controller.go
@@ -38,6 +38,9 @@ var (
 )
 
 // PodController specifies interface needed for starting, stopping pod and operations with it
+//
+// The purpose of this interface is to provide single mechanism of pod manipulation, reduce number of parameters which
+// caller needs to pass (since we keep pod related things internally) and eliminate human errors.
 type PodController interface {
 	PodName() string
 	Pod() *corev1.Pod
@@ -52,8 +55,9 @@ type PodController interface {
 	GetFileWriter() (PodFileWriter, error)
 }
 
-// podController specifies Kubernetes Client and PodOptions needed for creating
-// a Pod. It implements the podControllerProcessor interface.
+// podController keeps Kubernetes Client and PodOptions needed for creating a Pod.
+// It implements the PodControllerProcessor interface.
+// All communication with kubernetes API are done via PodControllerProcessor interface, which could be overriden for testing purposes.
 type podController struct {
 	cli        kubernetes.Interface
 	podOptions *PodOptions
@@ -67,7 +71,7 @@ type podController struct {
 
 type PodControllerOption func(p *podController)
 
-// WithPodControllerProcessor provides mechanism for passing fake podControllerProcessor for testing purposes.
+// WithPodControllerProcessor provides mechanism for passing fake implementation of PodControllerProcessor for testing purposes.
 func WithPodControllerProcessor(processor PodControllerProcessor) PodControllerOption {
 	return func(p *podController) {
 		p.pcp = processor
@@ -97,6 +101,7 @@ func NewPodController(cli kubernetes.Interface, options *PodOptions, opts ...Pod
 
 // NewPodControllerForExistingPod returns a new PodController given Kubernetes
 // Client and existing pod details.
+// Invocation of StartPod of returned PodController instance will fail, since the pod is already known.
 func NewPodControllerForExistingPod(cli kubernetes.Interface, pod *corev1.Pod) PodController {
 	r := &podController{
 		cli: cli,
@@ -147,7 +152,8 @@ func (p *podController) StartPod(ctx context.Context) error {
 	return nil
 }
 
-// WaitForPod waits for pod readiness.
+// WaitForPodReady waits for pod readiness (actually it waits until pod exit the pending state)
+// Requires pod to be started otherwise will immediately fail.
 func (p *podController) WaitForPodReady(ctx context.Context) error {
 	if p.podName == "" {
 		return ErrPodControllerPodNotStarted
@@ -163,7 +169,7 @@ func (p *podController) WaitForPodReady(ctx context.Context) error {
 	return nil
 }
 
-// WaitForPodCompletion waits for a pod to reach a terminal state.
+// WaitForPodCompletion waits for a pod to reach a terminal (either succeeded or failed) state.
 func (p *podController) WaitForPodCompletion(ctx context.Context) error {
 	if p.podName == "" {
 		return ErrPodControllerPodNotStarted
@@ -223,6 +229,8 @@ func (p *podController) getContainerName() string {
 	return p.pod.Spec.Containers[0].Name
 }
 
+// StreamPodLogs returns io.ReadCloser which could be used to receive logs from pod
+// Container will be decided based on the result of getContainerName function.
 func (p *podController) StreamPodLogs(ctx context.Context) (io.ReadCloser, error) {
 	if p.podName == "" {
 		return nil, ErrPodControllerPodNotStarted
@@ -231,6 +239,10 @@ func (p *podController) StreamPodLogs(ctx context.Context) (io.ReadCloser, error
 	return StreamPodLogs(ctx, p.cli, p.pod.Namespace, p.pod.Name, p.getContainerName())
 }
 
+// GetCommandExecutor returns PodCommandExecutor instance which is configured to execute commands within pod controlled
+// by this controller.
+// If pod is not created or not ready yet, it will fail with an appropriate error.
+// Container will be decided based on the result of getContainerName function
 func (p *podController) GetCommandExecutor() (PodCommandExecutor, error) {
 	if p.podName == "" {
 		return nil, ErrPodControllerPodNotStarted
@@ -254,6 +266,9 @@ func (p *podController) GetCommandExecutor() (PodCommandExecutor, error) {
 	return pce, nil
 }
 
+// GetFileWriter returns PodFileWriter instance which is configured to write file to pod controlled by this controller.
+// If pod is not created or not ready yet, it will fail with an appropriate error.
+// Container will be decided based on the result of getContainerName function
 func (p *podController) GetFileWriter() (PodFileWriter, error) {
 	if p.podName == "" {
 		return nil, ErrPodControllerPodNotStarted

diff --git a/pkg/kube/pod_file_writer.go b/pkg/kube/pod_file_writer.go
@@ -22,7 +22,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 )
 
-// PodFileRemover provides the mechanism to remove written file from the pod.
+// PodFileRemover provides mechanism for removing particular file written to the pod by PodFileWriter.
 type PodFileRemover interface {
 	Remove(ctx context.Context) error
 	Path() string
@@ -36,15 +36,18 @@ type podFileRemover struct {
 	path          string
 }
 
+// Remove deletes file from the pod.
 func (pfr *podFileRemover) Remove(ctx context.Context) error {
 	return pfr.podWriter.Remove(ctx, pfr.namespace, pfr.podName, pfr.containerName)
 }
 
+// Path returns path of the file within pod to be removed.
 func (pfr *podFileRemover) Path() string {
 	return pfr.path
 }
 
-// PodFileWriter allows us to write file to the pod.
+// PodFileWriter provides a way to write a file to the pod.
+// Is intended to be returned by PodController and works with pod controlled by it.
 type PodFileWriter interface {
 	Write(ctx context.Context, filePath string, content io.Reader) (PodFileRemover, error)
 }
@@ -59,8 +62,8 @@ type podFileWriter struct {
 	fileWriterProcessor PodFileWriterProcessor
 }
 
-// WriteFileToPod writes specified file content to a file in the pod and returns an interface
-// with which the file can be removed.
+// Write writes specified file content to a file in the pod and returns PodFileRemover
+// which should be used to remove written file.
 func (p *podFileWriter) Write(ctx context.Context, filePath string, content io.Reader) (PodFileRemover, error) {
 	pw := p.fileWriterProcessor.NewPodWriter(filePath, content)
 	if err := pw.Write(ctx, p.namespace, p.podName, p.containerName); err != nil {