Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: wip: fix render to deploy - DON'T Merge #7326

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
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
12 changes: 6 additions & 6 deletions cmd/skaffold/app/cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/spf13/cobra"

"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/tips"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
Expand All @@ -47,9 +46,6 @@ func NewCmdDeploy() *cobra.Command {

func doDeploy(ctx context.Context, out io.Writer) error {
return withRunner(ctx, out, func(r runner.Runner, configs []util.VersionedConfig) error {
if opts.SkipRender {
return r.DeployAndLog(ctx, out, []graph.Artifact{})
}
var artifacts []*latest.Artifact
for _, cfg := range configs {
artifacts = append(artifacts, cfg.(*latest.SkaffoldConfig).Build.Artifacts...)
Expand All @@ -59,7 +55,11 @@ func doDeploy(ctx context.Context, out io.Writer) error {
tips.PrintUseRunVsDeploy(out)
return err
}

return r.DeployAndLog(ctx, out, buildArtifacts)
// Render
manifests, errR := r.Render(ctx, out, buildArtifacts, true, opts.RenderOutput)
if errR != nil {
return errR
}
return r.DeployAndLog(ctx, out, buildArtifacts, manifests)
})
}
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ var flagRegistry = []Flag{
Value: &opts.Muted.Phases,
DefValue: []string{},
FlagAddMethod: "StringSliceVar",
DefinedOn: []string{"dev", "run", "debug", "build", "deploy"},
DefinedOn: []string{"dev", "run", "debug", "build", "render", "deploy"},
IsEnum: true,
},
{
Expand Down
4 changes: 2 additions & 2 deletions cmd/skaffold/app/cmd/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ func doRender(ctx context.Context, out io.Writer) error {
}
}

if err := r.Render(ctx, out, bRes, offline, opts.RenderOutput); err != nil {
return fmt.Errorf("rendering manifests: %w", err)
if _, errR := r.Render(ctx, out, bRes, offline, opts.RenderOutput); errR != nil {
return fmt.Errorf("rendering manifests: %w", errR)
}
return nil
})
Expand Down
8 changes: 7 additions & 1 deletion cmd/skaffold/app/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,13 @@ func doRun(ctx context.Context, out io.Writer) error {
}
}

err = r.DeployAndLog(ctx, out, bRes)
// Render
manifestList, err := r.Render(ctx, out, bRes, true, opts.RenderOutput)
if err != nil {
return err
}

err = r.DeployAndLog(ctx, out, bRes, manifestList)
if err != nil {
return fmt.Errorf("failed to deploy: %w", err)
}
Expand Down
9 changes: 8 additions & 1 deletion cmd/skaffold/app/cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext/v2"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
Expand All @@ -50,6 +51,7 @@ type mockRunRunner struct {
runner.Runner
testRan bool
deployRan bool
renderRan bool
artifactImageNames []string
}

Expand All @@ -71,11 +73,16 @@ func (r *mockRunRunner) Test(context.Context, io.Writer, []graph.Artifact) error
return nil
}

func (r *mockRunRunner) DeployAndLog(context.Context, io.Writer, []graph.Artifact) error {
func (r *mockRunRunner) DeployAndLog(context.Context, io.Writer, []graph.Artifact, manifest.ManifestList) error {
r.deployRan = true
return nil
}

func (r *mockRunRunner) Render(context.Context, io.Writer, []graph.Artifact, bool, string) (manifest.ManifestList, error) {
r.renderRan = true
return manifest.ManifestList{}, nil
}

func TestDoRun(t *testing.T) {
tests := []struct {
description string
Expand Down
2 changes: 2 additions & 0 deletions docs/content/en/docs/references/cli/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,7 @@ Options:
-l, --label=[]: Add custom labels to deployed objects. Set multiple times for multiple labels
--loud=false: Show the build logs and output
-m, --module=[]: Filter Skaffold configs to only the provided named modules
--mute-logs=[]: mute logs for specified stages in pipeline (build, deploy, status-check, none, all)
-n, --namespace='': Run deployments in the specified namespace
--offline=false: Do not connect to Kubernetes API server for manifest creation and validation. This is helpful when no Kubernetes cluster is available (e.g. GitOps model). No metadata.namespace attribute is injected in this case - the manifest content does not get changed.
-o, --output='': File to write rendered manifests to
Expand Down Expand Up @@ -986,6 +987,7 @@ Env vars:
* `SKAFFOLD_LABEL` (same as `--label`)
* `SKAFFOLD_LOUD` (same as `--loud`)
* `SKAFFOLD_MODULE` (same as `--module`)
* `SKAFFOLD_MUTE_LOGS` (same as `--mute-logs`)
* `SKAFFOLD_NAMESPACE` (same as `--namespace`)
* `SKAFFOLD_OFFLINE` (same as `--offline`)
* `SKAFFOLD_OUTPUT` (same as `--output`)
Expand Down
11 changes: 5 additions & 6 deletions examples/microservices/skaffold.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
apiVersion: skaffold/v2beta28
apiVersion: skaffold/v3alpha1
kind: Config
build:
artifacts:
Expand All @@ -14,11 +14,10 @@ build:
alias: BASE
- image: base
context: base
deploy:
kubectl:
manifests:
- leeroy-web/kubernetes/*
- leeroy-app/kubernetes/*
manifests:
rawYaml:
- leeroy-web/kubernetes/*
- leeroy-app/kubernetes/*
portForward:
- resourceType: deployment
resourceName: leeroy-web
Expand Down
68 changes: 34 additions & 34 deletions integration/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,16 @@ import (
"io/ioutil"
"os"
"path"
"path/filepath"
"regexp"
"testing"

yaml "gopkg.in/yaml.v2"

"github.com/GoogleContainerTools/skaffold/integration/skaffold"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/label"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/render/renderer/kubectl"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext/v2"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/testutil"
Expand Down Expand Up @@ -78,26 +76,21 @@ spec:
tmpDir := t.NewTempDir()
tmpDir.Write("deployment.yaml", test.input).Chdir()

deployer, err := kubectl.NewDeployer(&runcontext.RunContext{
WorkingDir: ".",
Pipelines: runcontext.NewPipelines([]latest.Pipeline{{
Render: latest.RenderConfig{
Generate: latest.Generate{
RawK8s: []string{"deployment.yaml"}},
},
}}),
}, &label.DefaultLabeller{}, &latest.KubectlDeploy{
Manifests: []string{"deployment.yaml"},
}, filepath.Join(tmpDir.Root(), test.renderPath))
mockCfg := mockConfig{
renderConfig: &latest.RenderConfig{
Generate: latest.Generate{
RawK8s: []string{"deployment.yaml"}},
},
workingDir: tmpDir.Root(),
}
r, err := kubectl.New(mockCfg, map[string]string{})
t.RequireNoError(err)
var b bytes.Buffer
err = deployer.Render(context.Background(), &b, test.builds, false, test.renderPath)
l, err := r.Render(context.Background(), &b, test.builds, false, test.renderPath)

t.CheckNoError(err)
dat, err := ioutil.ReadFile(test.renderPath)
t.CheckNoError(err)

t.CheckDeepEqual(test.expectedOut, string(dat))
t.CheckDeepEqual(test.expectedOut, l.String())
})
}

Expand Down Expand Up @@ -228,28 +221,24 @@ spec:
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.NewTempDir().
Write("deployment.yaml", test.input).
tmpDir := t.NewTempDir()
tmpDir.Write("deployment.yaml", test.input).
Chdir()

deployer, err := kubectl.NewDeployer(&runcontext.RunContext{
WorkingDir: ".",
Pipelines: runcontext.NewPipelines([]latest.Pipeline{{
Render: latest.RenderConfig{
Generate: latest.Generate{
RawK8s: []string{"deployment.yaml"}},
},
}}),
Opts: config.SkaffoldOptions{},
}, &label.DefaultLabeller{}, &latest.KubectlDeploy{
Manifests: []string{"deployment.yaml"},
}, "")
mockCfg := mockConfig{
renderConfig: &latest.RenderConfig{
Generate: latest.Generate{
RawK8s: []string{"deployment.yaml"}},
},
workingDir: tmpDir.Root(),
}
r, err := kubectl.New(mockCfg, map[string]string{})
t.RequireNoError(err)
var b bytes.Buffer
err = deployer.Render(context.Background(), &b, test.builds, false, "")
l, err := r.Render(context.Background(), &b, test.builds, false, "")

t.CheckNoError(err)
t.CheckDeepEqual(test.expectedOut, b.String())
t.CheckDeepEqual(test.expectedOut, l.String())
})
}
}
Expand Down Expand Up @@ -772,3 +761,14 @@ spec:
})
}
}

type mockConfig struct {
renderConfig *latest.RenderConfig
workingDir string
}

func (mc mockConfig) GetRenderConfig() *latest.RenderConfig { return mc.renderConfig }
func (mc mockConfig) GetWorkingDir() string { return mc.workingDir }
func (mc mockConfig) TransformAllowList() []latest.ResourceFilter { return nil }
func (mc mockConfig) TransformDenyList() []latest.ResourceFilter { return nil }
func (mc mockConfig) TransformRulesFile() string { return "" }
1 change: 1 addition & 0 deletions pkg/skaffold/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ func NewIntOrUndefined(v *int) IntOrUndefined {
}

func (m Muted) MuteBuild() bool { return m.mute("build") }
func (m Muted) MuteRender() bool { return m.mute("render") }
func (m Muted) MuteTest() bool { return m.mute("test") }
func (m Muted) MuteStatusCheck() bool { return m.mute("status-check") }
func (m Muted) MuteDeploy() bool { return m.mute("deploy") }
Expand Down
7 changes: 2 additions & 5 deletions pkg/skaffold/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/access"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/debug"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/log"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/status"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/sync"
Expand All @@ -33,7 +34,7 @@ import (
type Deployer interface {
// Deploy should ensure that the build results are deployed to the Kubernetes
// cluster.
Deploy(context.Context, io.Writer, []graph.Artifact) error
Deploy(context.Context, io.Writer, []graph.Artifact, manifest.ManifestList) error

// Dependencies returns a list of files that the deployer depends on.
// In dev mode, a redeploy will be triggered
Expand All @@ -42,10 +43,6 @@ type Deployer interface {
// Cleanup deletes what was deployed by calling Deploy.
Cleanup(context.Context, io.Writer, bool) error

// Render generates the Kubernetes manifests replacing the build results and
// writes them to the given file path
Render(context.Context, io.Writer, []graph.Artifact, bool, string) error

// GetDebugger returns a Deployer's implementation of a Debugger
GetDebugger() debug.Debugger

Expand Down
38 changes: 18 additions & 20 deletions pkg/skaffold/deploy/deploy_mux.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ limitations under the License.
package deploy

import (
"bytes"
"context"
"io"
"strconv"
"strings"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/access"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
Expand Down Expand Up @@ -105,7 +103,7 @@ func (m DeployerMux) RegisterLocalImages(images []graph.Artifact) {
}
}

func (m DeployerMux) Deploy(ctx context.Context, w io.Writer, as []graph.Artifact) error {
func (m DeployerMux) Deploy(ctx context.Context, w io.Writer, as []graph.Artifact, l manifest.ManifestList) error {
for i, deployer := range m.deployers {
eventV2.DeployInProgress(i)
w, ctx = output.WithEventContext(ctx, w, constants.Deploy, strconv.Itoa(i))
Expand All @@ -120,7 +118,7 @@ func (m DeployerMux) Deploy(ctx context.Context, w io.Writer, as []graph.Artifac
return err
}
}
if err := deployer.Deploy(ctx, w, as); err != nil {
if err := deployer.Deploy(ctx, w, as, l); err != nil {
eventV2.DeployFailed(i, err)
endTrace(instrumentation.TraceEndError(err))
return err
Expand Down Expand Up @@ -172,22 +170,22 @@ func (m DeployerMux) Cleanup(ctx context.Context, w io.Writer, dryRun bool) erro
return nil
}

func (m DeployerMux) Render(ctx context.Context, w io.Writer, as []graph.Artifact, offline bool, filepath string) error {
resources, buf := []string{}, &bytes.Buffer{}
for _, deployer := range m.deployers {
ctx, endTrace := instrumentation.StartTrace(ctx, "Render")
buf.Reset()
if err := deployer.Render(ctx, buf, as, offline, "" /* never write to files */); err != nil {
endTrace(instrumentation.TraceEndError(err))
return err
}
resources = append(resources, buf.String())
endTrace()
}

allResources := strings.Join(resources, "\n---\n")
return manifest.Write(strings.TrimSpace(allResources), filepath, w)
}
//func (m DeployerMux) Render(ctx context.Context, w io.Writer, as []graph.Artifact, offline bool, filepath string) error {
// resources, buf := []string{}, &bytes.Buffer{}
// for _, deployer := range m.deployers {
// ctx, endTrace := instrumentation.StartTrace(ctx, "Render")
// buf.Reset()
// if err := deployer.Render(ctx, buf, as, offline, "" /* never write to files */); err != nil {
// endTrace(instrumentation.TraceEndError(err))
// return err
// }
// resources = append(resources, buf.String())
// endTrace()
// }
//
// allResources := strings.Join(resources, "\n---\n")
// return manifest.Write(strings.TrimSpace(allResources), filepath, w)
//}

// TrackBuildArtifacts should *only* be called on individual deployers. This is a noop.
func (m DeployerMux) TrackBuildArtifacts(_ []graph.Artifact) {}
Loading