Skip to content

Commit

Permalink
Refactor resource interface
Browse files Browse the repository at this point in the history
Try to unify different types of pipeline resource when processing input
/ output resources. As we add more resources the contract should be more
simple.

Removed git from build source and add the functionality similar to other
resources. Resource defines list of container definitions to download
and list of container definitions to upload as well.
  • Loading branch information
shashwathi authored and Shash Reddy committed Feb 4, 2019
1 parent 7b5938b commit dce7b2f
Show file tree
Hide file tree
Showing 16 changed files with 512 additions and 899 deletions.
20 changes: 9 additions & 11 deletions cmd/git-init/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"flag"
"os"
"os/exec"
"path/filepath"

"github.com/knative/pkg/logging"
"go.uber.org/zap"
Expand Down Expand Up @@ -67,20 +66,19 @@ func main() {
if err != nil {
logger.Fatalf("Unexpected error creating symlink: %v", err)
}

dir, err := os.Getwd()
if err != nil {
logger.Errorf("Failed to get current dir", err)
if *revision == "" {
*revision = "master"
}

if *path != "" {
runOrFail(logger, "git", "init", *path)
path := filepath.Join(dir, *path)
if err := os.Chdir(path); err != nil {
if _, err := os.Stat(*path); os.IsNotExist(err) {
if err := os.Mkdir(*path, os.ModePerm); err != nil {
logger.Debugf("Creating directory at path %s", *path)
}
}
if err := os.Chdir(*path); err != nil {
logger.Fatalf("Failed to change directory with path %s; err %v", path, err)
}
// update dir variable with new path
dir = path
} else {
run(logger, "git", "init")
}
Expand All @@ -89,5 +87,5 @@ func main() {
runOrFail(logger, "git", "fetch", "--depth=1", "--recurse-submodules=yes", "origin", *revision)
runOrFail(logger, "git", "reset", "--hard", "FETCH_HEAD")

logger.Infof("Successfully cloned %q @ %q in path %q", *url, *revision, dir)
logger.Infof("Successfully cloned %q @ %q in path %q", *url, *revision, *path)
}
42 changes: 42 additions & 0 deletions pkg/apis/pipeline/v1alpha1/cluster_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,16 @@ package v1alpha1
import (
b64 "encoding/base64"
"encoding/json"
"flag"
"fmt"
"strconv"
"strings"

corev1 "k8s.io/api/core/v1"
)

var (
kubeconfigWriterImage = flag.String("kubeconfig-writer-image", "override-with-kubeconfig-writer:latest", "The container image containing our kubeconfig writer binary.")
)

// ClusterResource represents a cluster configuration (kubeconfig)
Expand Down Expand Up @@ -134,3 +141,38 @@ func (s ClusterResource) String() string {
json, _ := json.Marshal(s)
return string(json)
}

func (s *ClusterResource) GetUploadContainerSpec() ([]corev1.Container, error) {
return nil, nil
}

func (s *ClusterResource) SetDestinationDirectory(path string) {
}
func (s *ClusterResource) GetDownloadContainerSpec() ([]corev1.Container, error) {
var envVars []corev1.EnvVar
for _, sec := range s.Secrets {
ev := corev1.EnvVar{
Name: strings.ToUpper(sec.FieldName),
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: sec.SecretName,
},
Key: sec.SecretKey,
},
},
}
envVars = append(envVars, ev)
}

clusterContainer := corev1.Container{
Name: "kubeconfig",
Image: *kubeconfigWriterImage,
Args: []string{
"-clusterConfig", s.String(),
},
Env: envVars,
}

return []corev1.Container{clusterContainer}, nil
}
49 changes: 49 additions & 0 deletions pkg/apis/pipeline/v1alpha1/cluster_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -163,3 +164,51 @@ func TestNewClusterResource(t *testing.T) {
})
}
}

func Test_ClusterResource_GetDownloadContainerSpec(t *testing.T) {
testcases := []struct {
name string
clusterResource *ClusterResource
wantContainers []corev1.Container
wantErr bool
}{{
name: "valid cluster resource config",
clusterResource: &ClusterResource{
Name: "test-cluster-resource",
Type: PipelineResourceTypeCluster,
URL: "http://10.10.10.10",
Secrets: []SecretParam{{
FieldName: "cadata",
SecretKey: "cadatakey",
SecretName: "secret1",
}},
},
wantContainers: []corev1.Container{{
Name: "kubeconfig",
Image: "override-with-kubeconfig-writer:latest",
Args: []string{"-clusterConfig", `{"name":"test-cluster-resource","type":"cluster","url":"http://10.10.10.10","revision":"","username":"","password":"","token":"","Insecure":false,"cadata":null,"secrets":[{"fieldName":"cadata","secretKey":"cadatakey","secretName":"secret1"}]}`},
Env: []corev1.EnvVar{{
Name: "CADATA",
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "secret1",
},
Key: "cadatakey",
},
},
}},
}},
}}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
gotContainers, err := tc.clusterResource.GetDownloadContainerSpec()
if tc.wantErr && err == nil {
t.Fatalf("Expected error to be %t but got %v:", tc.wantErr, err)
}
if d := cmp.Diff(gotContainers, tc.wantContainers); d != "" {
t.Errorf("Error mismatch between download containers spec: %s", d)
}
})
}
}
15 changes: 8 additions & 7 deletions pkg/apis/pipeline/v1alpha1/gcs_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,12 @@ func (s *GCSResource) GetDownloadContainerSpec() ([]corev1.Container, error) {
}

envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts(s.Name, gcsSecretVolumeMountPath, s.Secrets)
return []corev1.Container{{
Name: fmt.Sprintf("storage-fetch-%s", s.Name),
Image: *gsutilImage,
Args: args,
Env: envVars,
VolumeMounts: secretVolumeMount,
}}, nil
return []corev1.Container{
CreateDirContainer(s.Name, s.DestinationDir), {
Name: fmt.Sprintf("storage-fetch-%s", s.Name),
Image: *gsutilImage,
Args: args,
Env: envVars,
VolumeMounts: secretVolumeMount,
}}, nil
}
50 changes: 26 additions & 24 deletions pkg/apis/pipeline/v1alpha1/gcs_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,19 +220,20 @@ func Test_GetDownloadContainerSpec(t *testing.T) {
SecretKey: "key.json",
}},
},
wantContainers: []corev1.Container{{
Name: "storage-fetch-gcs-valid",
Image: "override-with-gsutil-image:latest",
Args: []string{"-args", "cp -r gs://some-bucket/** /workspace"},
Env: []corev1.EnvVar{{
Name: "GOOGLE_APPLICATION_CREDENTIALS",
Value: "/var/secret/secretName/key.json",
}},
VolumeMounts: []corev1.VolumeMount{{
Name: "volume-gcs-valid-secretName",
MountPath: "/var/secret/secretName",
wantContainers: []corev1.Container{
CreateDirContainer("gcs-valid", "/workspace"), {
Name: "storage-fetch-gcs-valid",
Image: "override-with-gsutil-image:latest",
Args: []string{"-args", "cp -r gs://some-bucket/** /workspace"},
Env: []corev1.EnvVar{{
Name: "GOOGLE_APPLICATION_CREDENTIALS",
Value: "/var/secret/secretName/key.json",
}},
VolumeMounts: []corev1.VolumeMount{{
Name: "volume-gcs-valid-secretName",
MountPath: "/var/secret/secretName",
}},
}},
}},
}, {
name: "duplicate secret mount paths",
gcsResource: &GCSResource{
Expand All @@ -249,19 +250,20 @@ func Test_GetDownloadContainerSpec(t *testing.T) {
FieldName: "GOOGLE_APPLICATION_CREDENTIALS",
}},
},
wantContainers: []corev1.Container{{
Name: "storage-fetch-gcs-valid",
Image: "override-with-gsutil-image:latest",
Args: []string{"-args", "cp gs://some-bucket /workspace"},
Env: []corev1.EnvVar{{
Name: "GOOGLE_APPLICATION_CREDENTIALS",
Value: "/var/secret/secretName/key.json",
}},
VolumeMounts: []corev1.VolumeMount{{
Name: "volume-gcs-valid-secretName",
MountPath: "/var/secret/secretName",
wantContainers: []corev1.Container{
CreateDirContainer("gcs-valid", "/workspace"), {
Name: "storage-fetch-gcs-valid",
Image: "override-with-gsutil-image:latest",
Args: []string{"-args", "cp gs://some-bucket /workspace"},
Env: []corev1.EnvVar{{
Name: "GOOGLE_APPLICATION_CREDENTIALS",
Value: "/var/secret/secretName/key.json",
}},
VolumeMounts: []corev1.VolumeMount{{
Name: "volume-gcs-valid-secretName",
MountPath: "/var/secret/secretName",
}},
}},
}},
}, {
name: "invalid no destination directory set",
gcsResource: &GCSResource{
Expand Down
44 changes: 43 additions & 1 deletion pkg/apis/pipeline/v1alpha1/git_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,20 @@ limitations under the License.
package v1alpha1

import (
"flag"
"fmt"
"strings"

corev1 "k8s.io/api/core/v1"
)

const workspaceDir = "/workspace"

var (
gitSource = "git-source"
// The container with Git that we use to implement the Git source step.
gitImage = flag.String("git-image", "override-with-git:latest",
"The container image containing our Git binary.")
)

// GitResource is an endpoint from which to get data which is required
Expand All @@ -30,7 +42,8 @@ type GitResource struct {
// Git revision (branch, tag, commit SHA or ref) to clone. See
// https://git-scm.com/docs/gitrevisions#_specifying_revisions for more
// information.
Revision string `json:"revision"`
Revision string `json:"revision"`
TargetPath string
}

// NewGitResource create a new git resource to pass to Knative Build
Expand Down Expand Up @@ -84,3 +97,32 @@ func (s *GitResource) Replacements() map[string]string {
"revision": s.Revision,
}
}

func (s *GitResource) GetDownloadContainerSpec() ([]corev1.Container, error) {
args := []string{"-url", s.URL,
"-revision", s.Revision,
}
var dPath string
if s.TargetPath != "" {
dPath = s.TargetPath
} else {
dPath = s.Name
}

args = append(args, []string{"-path", dPath}...)

return []corev1.Container{{
Name: gitSource + "-" + s.Name,
Image: *gitImage,
Args: args,
WorkingDir: workspaceDir,
}}, nil
}

func (s *GitResource) SetDestinationDirectory(path string) {
s.TargetPath = path
}

func (s *GitResource) GetUploadContainerSpec() ([]corev1.Container, error) {
return nil, nil
}
11 changes: 11 additions & 0 deletions pkg/apis/pipeline/v1alpha1/image_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package v1alpha1
import (
"fmt"
"strings"

corev1 "k8s.io/api/core/v1"
)

// NewImageResource creates a new ImageResource from a PipelineResource.
Expand Down Expand Up @@ -73,3 +75,12 @@ func (s *ImageResource) Replacements() map[string]string {
"digest": s.Digest,
}
}

func (s *ImageResource) GetUploadContainerSpec() ([]corev1.Container, error) {
return nil, nil
}
func (s *ImageResource) GetDownloadContainerSpec() ([]corev1.Container, error) {
return nil, nil
}
func (s *ImageResource) SetDestinationDirectory(path string) {
}
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/knative/pkg/apis"
"github.com/knative/pkg/webhook"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -52,6 +53,9 @@ type PipelineResourceInterface interface {
GetType() PipelineResourceType
GetParams() []Param
Replacements() map[string]string
GetDownloadContainerSpec() ([]corev1.Container, error)
GetUploadContainerSpec() ([]corev1.Container, error)
SetDestinationDirectory(string)
}

// SecretParam indicates which secret can be used to populate a field of the resource
Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1alpha1/storage_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ package v1alpha1
import (
"fmt"
"strings"

corev1 "k8s.io/api/core/v1"
)

type PipelineResourceStorageType string
Expand All @@ -34,9 +32,6 @@ const (
type PipelineStorageResourceInterface interface {
PipelineResourceInterface
GetSecretParams() []SecretParam
GetDownloadContainerSpec() ([]corev1.Container, error)
GetUploadContainerSpec() ([]corev1.Container, error)
SetDestinationDirectory(string)
}

func NewStorageResource(r *PipelineResource) (PipelineStorageResourceInterface, error) {
Expand Down
Loading

0 comments on commit dce7b2f

Please sign in to comment.