Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions pkg/app/piped/cloudprovider/cloudrun/servicemanifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"os"
"strings"

"github.com/pipe-cd/pipecd/pkg/model"

@khanhtc1202 khanhtc1202 Mar 9, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits, lint error. Please refer to other files for importing conventions.

"google.golang.org/api/run/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -199,3 +200,34 @@ func parseContainerImage(image string) (name, tag string) {
name = paths[len(paths)-1]
return
}

func FindArtifactVersion(sm ServiceManifest) (*model.ArtifactVersion, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be unified let's how about returning a list of version.

Suggested change
func FindArtifactVersion(sm ServiceManifest) (*model.ArtifactVersion, error) {
func FindArtifactVersions(sm ServiceManifest) ([]model.ArtifactVersion, error) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds nice!
I did because Cloud Run can use only one image in one app.
So I'll fix to change returning a list of version and insert comment about that.

containers, ok, err := unstructured.NestedSlice(sm.u.Object, "spec", "template", "spec", "containers")
if err != nil {
return nil, err
}
if !ok || len(containers) == 0 {
return nil, fmt.Errorf("spec.template.spec.containers was missing")
}

container, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&containers[0])
if err != nil {
return nil, fmt.Errorf("invalid container format")
}

image, ok, err := unstructured.NestedString(container, "image")
if err != nil {
return nil, err
}
if !ok || image == "" {
return nil, fmt.Errorf("image was missing")
}
name, tag := parseContainerImage(image)

return &model.ArtifactVersion{
Kind: model.ArtifactVersion_CONTAINER_IMAGE,
Version: tag,
Name: name,
Url: image,
}, nil
}
130 changes: 130 additions & 0 deletions pkg/app/piped/cloudprovider/cloudrun/servicemanifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package cloudrun
import (
"testing"

"github.com/pipe-cd/pipecd/pkg/model"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -288,3 +289,132 @@ spec:
})
}
}

func TestFindArtifactVersion(t *testing.T) {
testcases := []struct {
name string
manifest string
want *model.ArtifactVersion
wantErr bool
}{
{
name: "ok",
manifest: `
apiVersion: serving.knative.dev/v1
kind: Service
metadata:
name: helloworld
labels:
cloud.googleapis.com/location: asia-northeast1
pipecd-dev-managed-by: piped
annotations:
run.googleapis.com/ingress: all
run.googleapis.com/ingress-status: all
spec:
template:
metadata:
name: helloworld-v010-1234567
annotations:
autoscaling.knative.dev/maxScale: '1'
spec:
containerConcurrency: 80
timeoutSeconds: 300
containers:
- image: gcr.io/pipecd/helloworld:v0.1.0
args:
- server
ports:
- name: http1
containerPort: 9085
resources:
limits:
cpu: 1000m
memory: 128Mi
traffic:
- revisionName: helloworld-v010-1234567
percent: 100
`,
want: &model.ArtifactVersion{
Kind: model.ArtifactVersion_CONTAINER_IMAGE,
Version: "v0.1.0",
Name: "helloworld",
Url: "gcr.io/pipecd/helloworld:v0.1.0",
},
wantErr: false,
},
{
name: "err: containers missing",
manifest: `
apiVersion: serving.knative.dev/v1
kind: Service
metadata:
name: helloworld
spec:
template:
metadata:
name: helloworld-v010-1234567
annotations:
autoscaling.knative.dev/maxScale: '1'
spec:
containerConcurrency: 80
timeoutSeconds: 300
`,
want: nil,
wantErr: true,
},
{
name: "err: image missing",
manifest: `
apiVersion: serving.knative.dev/v1
kind: Service
metadata:
name: helloworld
labels:
cloud.googleapis.com/location: asia-northeast1
pipecd-dev-managed-by: piped
annotations:
run.googleapis.com/ingress: all
run.googleapis.com/ingress-status: all
spec:
template:
metadata:
name: helloworld-v010-1234567
annotations:
autoscaling.knative.dev/maxScale: '1'
spec:
containerConcurrency: 80
timeoutSeconds: 300
containers:
- args:
- server
ports:
- name: http1
containerPort: 9085
resources:
limits:
cpu: 1000m
memory: 128Mi
traffic:
- revisionName: helloworld-v010-1234567
percent: 100
`,
want: nil,
wantErr: true,
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
data := []byte(tc.manifest)
sm, err := ParseServiceManifest(data)
require.NoError(t, err)

got, err := FindArtifactVersion(sm)
if tc.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, tc.want, got)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits

Suggested change
if tc.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, tc.want, got)
assert.Equal(t, tc.wantErr, err != nil)
assert.Equal(t, tc.want, got)

})
}
}
21 changes: 21 additions & 0 deletions pkg/app/piped/planner/cloudrun/cloudrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu
in.Logger.Warn("unable to determine target version", zap.Error(e))
}

if av, e := p.determineArtifactVersion(ds.AppDir, cfg.Input.ServiceManifestFile); e == nil {
out.Versions = []*model.ArtifactVersion{av}
} else {
in.Logger.Warn("unable to determine target versions", zap.Error(e))
out.Versions = []*model.ArtifactVersion{
{
Kind: model.ArtifactVersion_UNKNOWN,
Version: "unknown",
},
}
}

@knanao knanao Mar 9, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits

Suggested change
if av, e := p.determineArtifactVersion(ds.AppDir, cfg.Input.ServiceManifestFile); e == nil {
out.Versions = []*model.ArtifactVersion{av}
} else {
in.Logger.Warn("unable to determine target versions", zap.Error(e))
out.Versions = []*model.ArtifactVersion{
{
Kind: model.ArtifactVersion_UNKNOWN,
Version: "unknown",
},
}
}
av, err := p.determineArtifactVersion(ds.AppDir, cfg.Input.ServiceManifestFile)
if err != nil {
in.Logger.Warn("unable to determine target versions", zap.Error(e))
av = &model.ArtifactVersion{
Kind: model.ArtifactVersion_UNKNOWN,
Version: "unknown",
}
}
out.Versions = []*model.ArtifactVersion{av}

autoRollback := *cfg.Input.AutoRollback

// In case the strategy has been decided by trigger.
Expand Down Expand Up @@ -133,3 +145,12 @@ func (p *Planner) determineVersion(appDir, serviceManifestFile string) (string,

return provider.FindImageTag(sm)
}

func (p *Planner) determineArtifactVersion(appDir, serviceManifestFile string) (*model.ArtifactVersion, error) {
sm, err := provider.LoadServiceManifest(appDir, serviceManifestFile)
if err != nil {
return nil, err
}

return provider.FindArtifactVersion(sm)
}