Skip to content

Commit

Permalink
Add and fix various docstrings and comments ✍️
Browse files Browse the repository at this point in the history
While working on tektoncd#1417, which we decided in the end not to merge, I made
a bunch of updates to docstrings and comments which are now in this
commit:
- Adding more detail to some docstrings
- Adding missing docstrings
- Updating docstrings that were sometimes wrong (e.g. a function name
  had changed)
- Removing some comments in the Conditions logic that were no longer
  accurate
- Added comments in some sections of code that took me a while to
  understand to hopefully make it faster next time :)
  • Loading branch information
bobcatfish committed Oct 25, 2019
1 parent 1271c3f commit 5716cf7
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 20 deletions.
16 changes: 8 additions & 8 deletions pkg/apis/pipeline/v1alpha1/artifact_pvc.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,25 @@ var (
pvcDir = "/pvc"
)

// ArtifactPVC represents the pvc created by the pipelinerun
// for artifacts temporary storage
// ArtifactPVC represents the pvc created by the pipelinerun for artifacts temporary storage.
type ArtifactPVC struct {
Name string
PersistentVolumeClaim *corev1.PersistentVolumeClaim

BashNoopImage string
}

// GetType returns the type of the artifact storage
// GetType returns the type of the artifact storage.
func (p *ArtifactPVC) GetType() string {
return ArtifactStoragePVCType
}

// StorageBasePath returns the path to be used to store artifacts in a pipelinerun temporary storage
// StorageBasePath returns the path to be used to store artifacts in a pipelinerun temporary storage.
func (p *ArtifactPVC) StorageBasePath(pr *PipelineRun) string {
return pvcDir
}

// GetCopyFromStorageToSteps returns a container used to download artifacts from temporary storage
// GetCopyFromStorageToSteps returns a container used to download artifacts from temporary storage.
func (p *ArtifactPVC) GetCopyFromStorageToSteps(name, sourcePath, destinationPath string) []Step {
return []Step{{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("source-copy-%s", name)),
Expand All @@ -57,7 +56,7 @@ func (p *ArtifactPVC) GetCopyFromStorageToSteps(name, sourcePath, destinationPat
}}}
}

// GetCopyToStorageFromSteps returns a container used to upload artifacts for temporary storage
// GetCopyToStorageFromSteps returns a container used to upload artifacts for temporary storage.
func (p *ArtifactPVC) GetCopyToStorageFromSteps(name, sourcePath, destinationPath string) []Step {
return []Step{{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("source-mkdir-%s", name)),
Expand All @@ -78,15 +77,16 @@ func (p *ArtifactPVC) GetCopyToStorageFromSteps(name, sourcePath, destinationPat
}}}
}

// GetPvcMount returns a mounting of the volume with the mount path /pvc
// GetPvcMount returns a mounting of the volume with the mount path /pvc.
func GetPvcMount(name string) corev1.VolumeMount {
return corev1.VolumeMount{
Name: name, // taskrun pvc name
MountPath: pvcDir, // nothing should be mounted here
}
}

// CreateDirStep returns a container step to create a dir
// CreateDirStep returns a container step to create a dir at destinationPath. The name
// of the step will include name.
func CreateDirStep(bashNoopImage string, name, destinationPath string) Step {
return Step{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("create-dir-%s", strings.ToLower(name))),
Expand Down
3 changes: 1 addition & 2 deletions pkg/apis/pipeline/v1alpha1/build_gcs_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ var validArtifactTypes = []GCSArtifactType{
// BuildGCSResource describes a resource in the form of an archive,
// or a source manifest describing files to fetch.
// BuildGCSResource does incremental uploads for files in directory.

type BuildGCSResource struct {
Name string
Type PipelineResourceType
Expand All @@ -70,7 +69,7 @@ type BuildGCSResource struct {
BuildGCSFetcherImage string `json:"-"`
}

// creates a new BuildGCS resource to pass to a Task
// NewBuildGCSResource creates a new BuildGCS resource to pass to a Task.
func NewBuildGCSResource(images pipeline.Images, r *PipelineResource) (*BuildGCSResource, error) {
if r.Spec.Type != PipelineResourceTypeStorage {
return nil, xerrors.Errorf("BuildGCSResource: Cannot create a BuildGCS resource from a %s Pipeline Resource", r.Spec.Type)
Expand Down
12 changes: 11 additions & 1 deletion pkg/apis/pipeline/v1alpha1/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,18 @@ var AllResourceTypes = []PipelineResourceType{PipelineResourceTypeGit, PipelineR

// PipelineResourceInterface interface to be implemented by different PipelineResource types
type PipelineResourceInterface interface {
// GetName returns the name of this PipelineResource instance.
GetName() string
// GetType returns the type of this PipelineResource (often a super type, e.g. in the case of storage).
GetType() PipelineResourceType
// Replacements returns all the attributes that this PipelineResource has that
// can be used for variable replacement.
Replacements() map[string]string
// GetOutputTaskModifier returns the TaskModifier instance that should be used on a Task
// in order to add this kind of resource when it is being used as an output.
GetOutputTaskModifier(ts *TaskSpec, path string) (TaskModifier, error)
// GetInputTaskModifier returns the TaskModifier instance that should be used on a Task
// in order to add this kind of resource when it is being used as an input.
GetInputTaskModifier(ts *TaskSpec, path string) (TaskModifier, error)
}

Expand Down Expand Up @@ -201,7 +209,9 @@ type ResourceDeclaration struct {
TargetPath string `json:"targetPath,omitempty"`
}

// ResourceFromType returns a PipelineResourceInterface from a PipelineResource's type.
// ResourceFromType returns an instance of the correct PipelineResource object type which can be
// used to add input and ouput containers as well as volumes to a TaskRun's pod in order to realize
// a PipelineResource in a pod.
func ResourceFromType(r *PipelineResource, images pipeline.Images) (PipelineResourceInterface, error) {
switch r.Spec.Type {
case PipelineResourceTypeGit:
Expand Down
10 changes: 8 additions & 2 deletions pkg/apis/pipeline/v1alpha1/storage_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,23 @@ import (
type PipelineResourceStorageType string

const (
// PipelineResourceTypeGCS indicates that resource source is a GCS blob/directory.
// PipelineResourceTypeGCS is the subtype for the GCSResources, which is backed by a GCS blob/directory.
PipelineResourceTypeGCS PipelineResourceType = "gcs"
// PipelineResourceTypeBuildGCS is the subtype for the BuildGCSResources, which is simialr to the GCSResource but
// with additional funcitonality that was added to be compatible with knative build.
PipelineResourceTypeBuildGCS PipelineResourceType = "build-gcs"
)

// PipelineResourceInterface interface to be implemented by different PipelineResource types
// PipelineStorageResourceInterface is the interface for subtypes of the storage type.
// It adds a function to the PipelineResourceInterface for retrieving secrets that are usually
// needed for storage PipelineResources.
type PipelineStorageResourceInterface interface {
PipelineResourceInterface
GetSecretParams() []SecretParam
}

// NewStorageResource returns an instance of the requested storage subtype, which can be used
// to add input and output steps and volumes to an executing pod.
func NewStorageResource(images pipeline.Images, r *PipelineResource) (PipelineStorageResourceInterface, error) {
if r.Spec.Type != PipelineResourceTypeStorage {
return nil, xerrors.Errorf("StoreResource: Cannot create a storage resource from a %s Pipeline Resource", r.Spec.Type)
Expand Down
9 changes: 3 additions & 6 deletions pkg/reconciler/pipelinerun/resources/conditionresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,7 @@ func (rcc *ResolvedConditionCheck) ConditionToTaskSpec() (*v1alpha1.TaskSpec, er
})
}

// convert param strings of type $(params.x) to $(inputs.params.x)
convertParamTemplates(&t.Steps[0], rcc.Condition.Spec.Params)
// convert resource strings of type $(resources.name.key) to $(inputs.resources.name.key)
err := ApplyResourceSubstitution(&t.Steps[0], rcc.ResolvedResources, rcc.Condition.Spec.Resources, rcc.images)

if err != nil {
Expand All @@ -122,7 +120,7 @@ func (rcc *ResolvedConditionCheck) ConditionToTaskSpec() (*v1alpha1.TaskSpec, er
return t, nil
}

// Replaces all instances of $(params.x) in the container to $(inputs.params.x) for each param name
// convertParamTemplates replaces all instances of $(params.x) in the container to $(inputs.params.x) for each param name.
func convertParamTemplates(step *v1alpha1.Step, params []v1alpha1.ParamSpec) {
replacements := make(map[string]string)
for _, p := range params {
Expand All @@ -133,8 +131,7 @@ func convertParamTemplates(step *v1alpha1.Step, params []v1alpha1.ParamSpec) {
v1alpha1.ApplyStepReplacements(step, replacements, map[string][]string{})
}

// ApplyResources applies the substitution from values in resources which are referenced
// in spec as subitems of the replacementStr.
// ApplyResourceSubstitution applies resource attribute variable substitution.
func ApplyResourceSubstitution(step *v1alpha1.Step, resolvedResources map[string]*v1alpha1.PipelineResource, conditionResources []v1alpha1.ResourceDeclaration, images pipeline.Images) error {
replacements := make(map[string]string)
for _, cr := range conditionResources {
Expand All @@ -153,7 +150,7 @@ func ApplyResourceSubstitution(step *v1alpha1.Step, resolvedResources map[string
return nil
}

// NewConditionCheck status creates a ConditionCheckStatus from a ConditionCheck
// NewConditionCheckStatus creates a ConditionCheckStatus from a ConditionCheck
func (rcc *ResolvedConditionCheck) NewConditionCheckStatus() *v1alpha1.ConditionCheckStatus {
var checkStep corev1.ContainerState
trs := rcc.ConditionCheck.Status
Expand Down
4 changes: 3 additions & 1 deletion pkg/reconciler/pipelinerun/resources/input_output_steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
)

// GetOutputSteps will add the correct `path` to the input resources for pt
// GetOutputSteps will add the correct `path` to the output resources for pt
func GetOutputSteps(outputs map[string]*v1alpha1.PipelineResource, taskName, storageBasePath string) []v1alpha1.TaskResourceBinding {
var taskOutputResources []v1alpha1.TaskResourceBinding

Expand Down Expand Up @@ -78,6 +78,8 @@ func GetInputSteps(inputs map[string]*v1alpha1.PipelineResource, pt *v1alpha1.Pi
}
}

// Determine if the value is meant to come `from` a previous Task - if so, add the path to the pvc
// that contains the data as the `path` the resulting TaskRun should get the data from.
var stepSourceNames []string
if pt.Resources != nil {
for _, pipelineTaskInput := range pt.Resources.Inputs {
Expand Down
1 change: 1 addition & 0 deletions pkg/reconciler/taskrun/resources/output_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func AddOutputResources(
}
v1alpha1.ApplyTaskModifier(taskSpec, modifier)

// Attach the PVC that will be used for `from` copying.
if as.GetType() == v1alpha1.ArtifactStoragePVCType {
if pvcName == "" {
return taskSpec, nil
Expand Down

0 comments on commit 5716cf7

Please sign in to comment.