Migrate invocation image push to `porter publish` #365

jeremyrickard · 2019-05-30T16:05:32Z

This PR migrates the image push functionality to a new command porter publish (the command doesn't actually publish the bundle itself to OCI yet). Now when you do a porter build the invocation image is still built, but it is not pushed to a registry. The bundle.json is generated with whatever tag is present in the porter.yaml.

To push the image and regenerate the bundle.json, you now do a porter publish.

Command outputs below:

porter build
Copying dependencies ===>
Copying porter runtime ===>
Copying mixins ===>
Copying mixin kubernetes ===>

Generating Dockerfile =======>
FROM quay.io/deis/lightweight-docker-go:v0.2.0
FROM debian:stretch
COPY --from=0 /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
RUN apt-get update && \
apt-get install -y apt-transport-https curl && \
curl -o kubectl https://storage.googleapis.com/kubernetes-release/release/v1.13.0/bin/linux/amd64/kubectl && \
mv kubectl /usr/local/bin && \
chmod a+x /usr/local/bin/kubectl

COPY cnab/ /cnab/
COPY porter.yaml /cnab/app/porter.yaml
CMD ["/cnab/app/run"]

Writing Dockerfile =======>
FROM quay.io/deis/lightweight-docker-go:v0.2.0
FROM debian:stretch
COPY --from=0 /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
RUN apt-get update && \
apt-get install -y apt-transport-https curl && \
curl -o kubectl https://storage.googleapis.com/kubernetes-release/release/v1.13.0/bin/linux/amd64/kubectl && \
mv kubectl /usr/local/bin && \
chmod a+x /usr/local/bin/kubectl

COPY cnab/ /cnab/
COPY porter.yaml /cnab/app/porter.yaml
CMD ["/cnab/app/run"]

Starting Invocation Image Build =======>
Step 1/7 : FROM quay.io/deis/lightweight-docker-go:v0.2.0
 ---> acf6712d2918
Step 2/7 : FROM debian:stretch
 ---> de8b49d4b0b3
Step 3/7 : COPY --from=0 /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
 ---> Using cache
 ---> 6bf0ab0d751e
Step 4/7 : RUN apt-get update && apt-get install -y apt-transport-https curl && curl -o kubectl https://storage.googleapis.com/kubernetes-release/release/v1.13.0/bin/linux/amd64/kubectl && mv kubectl /usr/local/bin && chmod a+x /usr/local/bin/kubectl
 ---> Using cache
 ---> 1fd544730ffe
Step 5/7 : COPY cnab/ /cnab/
 ---> ba4fdfd5ef4b
Step 6/7 : COPY porter.yaml /cnab/app/porter.yaml
 ---> eb4a04350e4a
Step 7/7 : CMD ["/cnab/app/run"]
 ---> Running in 043633b8fcca
 ---> 49aaf30f0465
Successfully built 49aaf30f0465
Successfully tagged jeremyrickard/porter-kubernetes:latest

Generating Bundle File with Invocation Image jeremyrickard/porter-kubernetes:latest =======>
Generating parameter definition porter-debug ====>
Generating credential kubeconfig ====>

Then a porter publish

porter publish --tag jeremyrickard/hashi-mashi:v1.0
Pushing CNAB invocation image...
The push refers to repository [docker.io/jeremyrickard/hashi-mashi]
15efbe06773e: Preparing
c7fa3bae30d9: Preparing
49037d9d1b30: Preparing
c7956a703d1e: Preparing
c581f4ede92d: Preparing
15efbe06773e: Mounted from jeremyrickard/porter-kubernetes
c581f4ede92d: Mounted from jeremyrickard/spring-music-installer
c7fa3bae30d9: Mounted from jeremyrickard/porter-kubernetes
c7956a703d1e: Mounted from jeremyrickard/porter-azure-wordpress
49037d9d1b30: Mounted from jeremyrickard/porter-kubernetes
v1.0: digest: sha256:c90044b7bfdab915d213ceccdce3910f51e5815a4dae72592fa357e2044c9717 size: 1370

Generating CNAB bundle.json...

Generating Bundle File with Invocation Image jeremyrickard/hashi-mashi@sha256:c90044b7bfdab915d213ceccdce3910f51e5815a4dae72592fa357e2044c9717 =======>
Generating parameter definition porter-debug ====>
Generating credential kubeconfig ====>

Closes: #354

Closes: getporter#354

jeremyrickard · 2019-05-30T16:05:47Z

Need to update the docs still!

carolynvs · 2019-05-30T16:50:26Z

cmd/porter/bundle.go

+	cmd := cobra.Command{
+		Use:   "publish",
+		Short: "Publish a bundle",
+		Long:  "Publishes a bundle by pushing the invocation image, updating the reference in the CNAB bundle.json, and publishing to an OCI registry.",


We don't normally put the technical implementation into the help text. I recommend putting this into the documentation, and then putting a more general description here like

"Publishes a bundle by pushing the invocation image and bundle to a registry"

carolynvs · 2019-05-30T16:55:57Z

cmd/porter/bundle.go

+
+	f := cmd.Flags()
+	f.StringVarP(&opts.File, "file", "f", "", "Path to the Porter manifest. Defaults to `porter.yaml` in the current directory.")
+	f.StringVarP(&opts.Tag, "tag", "t", "", "Tag to apply to the CNAB invocation image. Defaults to the value currently in the Porter manifest.")


This isn't what was specified in the OP issue #254. That was calling for a new field tag (format registry/image:tag) to be added to the manifest that the bundle will be pushed to).

So this should say

"Tag to apply to the bundle when pushing to the registry. Defaults to the value defined in the Porter manifest."

👍 i misread that.

carolynvs · 2019-05-30T17:00:43Z

pkg/porter/publish.go

+	if err != nil {
+		return errors.Wrap(err, "could not get current working directory")
+	}
+	fmt.Println(pwd)


Looks like a leftover debug statement that should be removed?

carolynvs · 2019-05-30T17:02:00Z

pkg/porter/publish.go

+		return errors.Wrap(err, "could not get current working directory")
+	}
+	fmt.Println(pwd)
+	files, err := fs.ReadDir(pwd)


Why not use fs.Exists to check if porter.yaml is in the current directory?

I copied this from the existing validation code (that wasn't directly reusable) and then removed something and was lazy. I'll update.

carolynvs · 2019-05-30T17:03:52Z

pkg/porter/publish.go

+
+func (p PublishOptions) Validate(porter *Porter) error {
+	if p.File != "" {
+		return nil


This is skipping the Tag validation entirely. I think the logic below around checking if porter.yaml is present should be in the if/else instead of doing a hard return, no?

carolynvs · 2019-05-30T17:20:46Z

pkg/porter/publish.go

+	if err != nil {
+		return err
+	}
+	if opts.Tag != "" {


Yeah this entire part I think should be removed since tag = bundle tag, not invocation image tag. Feel free to give things better names anywhere you like. 😀

I think we should still validate that the tag given is a valid OCI tag no?

Yes, in the PublishOptions.Validate method. I was just referring to the lines 81-83 here where we are changing the invocation image tag.

carolynvs · 2019-05-30T17:24:39Z

pkg/porter/publish_test.go

+	opts.Tag = "someinvalid/repo/thing:10:10"
+
+	err = opts.Validate(p.Porter)
+	require.Error(t, err, "should have no error")


the error message here is confusing, can you remove it or fix it? We actually do want an error.

carolynvs · 2019-05-30T17:24:44Z

pkg/porter/publish_test.go

+	p.TestConfig.SetupPorterHome()
+	opts := PublishOptions{}
+	err := opts.Validate(p.Porter)
+	require.Error(t, err, "should have no error")


the error message here is confusing, can you remove it or fix it? We actually do want an error.

carolynvs

sorry just one nit and then the rubber stamps will fly! 💸

carolynvs · 2019-05-30T17:59:49Z

pkg/porter/publish.go

 	if p.Tag != "" {
 		_, err := reference.ParseNormalizedNamed(p.Tag)
 		if err != nil {
-			return err
+			return errors.Wrap(err, "invalid bundle tag value")


nit: Normally in the validate methods we tell them the flag that they need to fix, so the error looks like "invalid --tag value, expected format is REGISTRY/IMAGE:TAG" or something along those lines.

$ porter publish --tag jeremyrickard/super-cool-app-image:v1.0:1010 Error: invalid --tag value. expected format is REGISTRY/IMAGE:TAG: invalid reference format

carolynvs

Migrate invocation image push to porter publish

e17f195

Closes: getporter#354

jeremyrickard self-assigned this May 30, 2019

jeremyrickard added 3 commits May 30, 2019 10:33

Fixed a test oversight

d600d4b

Udpated docs

b8efba7

YOU TAGGED THE WRONG THING JEREMY

5ec93b2

carolynvs requested changes May 30, 2019

View reviewed changes

jeremyrickard added 2 commits May 30, 2019 11:48

Resolve review comments

19b57d8

Wrap the error in validation

9dea3f1

jeremyrickard requested a review from carolynvs May 30, 2019 17:57

carolynvs requested changes May 30, 2019

View reviewed changes

Update error message

55c9060

carolynvs approved these changes May 30, 2019

View reviewed changes

jeremyrickard merged commit 75bb688 into getporter:master May 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate invocation image push to `porter publish` #365

Migrate invocation image push to `porter publish` #365

jeremyrickard commented May 30, 2019

jeremyrickard commented May 30, 2019

carolynvs May 30, 2019

carolynvs May 30, 2019

jeremyrickard May 30, 2019

carolynvs May 30, 2019

carolynvs May 30, 2019

jeremyrickard May 30, 2019 •

edited

Loading

carolynvs May 30, 2019

carolynvs May 30, 2019

jeremyrickard May 30, 2019

carolynvs May 30, 2019

carolynvs May 30, 2019

carolynvs May 30, 2019

carolynvs left a comment

carolynvs May 30, 2019

jeremyrickard May 30, 2019

carolynvs left a comment

Migrate invocation image push to porter publish #365

Migrate invocation image push to porter publish #365

Conversation

jeremyrickard commented May 30, 2019

jeremyrickard commented May 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremyrickard May 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carolynvs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carolynvs left a comment

Choose a reason for hiding this comment

Migrate invocation image push to `porter publish` #365

Migrate invocation image push to `porter publish` #365

jeremyrickard May 30, 2019 •

edited

Loading