From 42456b29079d90c118bf420450f427abf9adfea4 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 1 Mar 2023 09:19:01 +0100 Subject: [PATCH] fix issues from manual testing and remote deploy (#3411) (#3413) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: remote destroy parsing Signed-off-by: Javier López Barba * feat: destroy marshalling Signed-off-by: Javier López Barba * fix: extertnals not going throught proxy Signed-off-by: Javier López Barba * fix: logger Signed-off-by: Javier López Barba * fix: unit tests Signed-off-by: Javier López Barba * address feedback Signed-off-by: Javier López Barba --------- Signed-off-by: Javier López Barba (cherry picked from commit d1f141f436020e53aee4ecbed82a0a36b0b4748e) Co-authored-by: Javier López Barba --- cmd/deploy/deploy.go | 6 +- cmd/deploy/deploy_test.go | 6 +- cmd/deploy/endpoints.go | 14 ++- cmd/deploy/local.go | 25 +++-- pkg/cmd/build/logger.go | 3 +- pkg/model/manifest.go | 7 ++ pkg/model/serializer.go | 55 ++++++++++ pkg/model/serializer_test.go | 205 +++++++++++++++++++++++++++++++++++ 8 files changed, 306 insertions(+), 15 deletions(-) diff --git a/cmd/deploy/deploy.go b/cmd/deploy/deploy.go index ba3415ac0e05..f5db76320f50 100644 --- a/cmd/deploy/deploy.go +++ b/cmd/deploy/deploy.go @@ -90,10 +90,14 @@ type DeployCommand struct { isRemote bool } +type ExternalResourceValidatorInterface interface { + Validate(ctx context.Context, name string, ns string, externalInfo *externalresource.ExternalResource) error +} + type ExternalResourceInterface interface { Deploy(ctx context.Context, name string, ns string, externalInfo *externalresource.ExternalResource) error List(ctx context.Context, ns string, labelSelector string) ([]externalresource.ExternalResource, error) - Validate(ctx context.Context, name string, ns string, externalInfo *externalresource.ExternalResource) error + ExternalResourceValidatorInterface } type deployerInterface interface { diff --git a/cmd/deploy/deploy_test.go b/cmd/deploy/deploy_test.go index 76101bb2b7ed..eac529c82517 100644 --- a/cmd/deploy/deploy_test.go +++ b/cmd/deploy/deploy_test.go @@ -734,6 +734,10 @@ func (f *fakeExternalControlProvider) getFakeExternalControl(cp okteto.K8sClient return f.control, f.err } +func (f *fakeExternalControlProvider) getFakeExternalControlValidator(cp okteto.K8sClientProvider) (ExternalResourceValidatorInterface, error) { + return f.control, f.err +} + func TestDeployExternals(t *testing.T) { ctx := context.Background() okteto.CurrentStore = &okteto.OktetoContextStore{ @@ -915,7 +919,7 @@ func TestValidateK8sResources(t *testing.T) { } ld := localDeployer{ - GetExternalControl: cp.getFakeExternalControl, + GetExternalControlForValidator: cp.getFakeExternalControlValidator, } if tc.expectedErr { diff --git a/cmd/deploy/endpoints.go b/cmd/deploy/endpoints.go index 51f060ff37ea..1631e571ac7f 100644 --- a/cmd/deploy/endpoints.go +++ b/cmd/deploy/endpoints.go @@ -28,6 +28,7 @@ import ( "github.com/okteto/okteto/pkg/externalresource" k8sExternalResources "github.com/okteto/okteto/pkg/externalresource/k8s" "github.com/okteto/okteto/pkg/format" + kconfig "github.com/okteto/okteto/pkg/k8s/kubeconfig" oktetoLog "github.com/okteto/okteto/pkg/log" "github.com/okteto/okteto/pkg/model" "github.com/okteto/okteto/pkg/okteto" @@ -228,8 +229,19 @@ func (dc *endpointGetter) showEndpoints(ctx context.Context, opts *EndpointsOpti return nil } +func getExternalControlForValidator(cp okteto.K8sClientProvider) (ExternalResourceValidatorInterface, error) { + _, cfg, err := cp.Provide(okteto.Context().Cfg) + if err != nil { + return nil, err + } + return &externalresource.K8sControl{ + ClientProvider: k8sExternalResources.GetExternalClient, + Cfg: cfg, + }, nil +} + func getExternalControlFromCtx(cp okteto.K8sClientProvider, filename string) (ExternalResourceInterface, error) { - _, proxyConfig, err := cp.Provide(okteto.Context().Cfg) + _, proxyConfig, err := cp.Provide(kconfig.Get([]string{filename})) if err != nil { return nil, err } diff --git a/cmd/deploy/local.go b/cmd/deploy/local.go index 6b2bbe8afa1c..b720b067ea7a 100644 --- a/cmd/deploy/local.go +++ b/cmd/deploy/local.go @@ -44,7 +44,9 @@ type localDeployer struct { Executor executor.ManifestExecutor TempKubeconfigFile string K8sClientProvider okteto.K8sClientProvider - GetExternalControl func(cp okteto.K8sClientProvider, filename string) (ExternalResourceInterface, error) + + GetExternalControlForValidator func(cp okteto.K8sClientProvider) (ExternalResourceValidatorInterface, error) + GetExternalControl func(cp okteto.K8sClientProvider, filename string) (ExternalResourceInterface, error) cwd string deployWaiter deployWaiter @@ -82,15 +84,16 @@ func newLocalDeployer(ctx context.Context, cwd string, options *Options) (*local clientProvider := okteto.NewK8sClientProvider() return &localDeployer{ - Kubeconfig: kubeconfig, - Executor: executor.NewExecutor(oktetoLog.GetOutputFormat(), options.RunWithoutBash), - Proxy: proxy, - TempKubeconfigFile: GetTempKubeConfigFile(tempKubeconfigName), - K8sClientProvider: clientProvider, - GetExternalControl: getExternalControlFromCtx, - deployWaiter: newDeployWaiter(clientProvider), - isRemote: true, - Fs: afero.NewOsFs(), + Kubeconfig: kubeconfig, + Executor: executor.NewExecutor(oktetoLog.GetOutputFormat(), options.RunWithoutBash), + Proxy: proxy, + TempKubeconfigFile: GetTempKubeConfigFile(tempKubeconfigName), + K8sClientProvider: clientProvider, + GetExternalControlForValidator: getExternalControlForValidator, + GetExternalControl: getExternalControlFromCtx, + deployWaiter: newDeployWaiter(clientProvider), + isRemote: true, + Fs: afero.NewOsFs(), }, nil } @@ -385,7 +388,7 @@ func (ld *localDeployer) validateK8sResources(ctx context.Context, manifest *mod // In a cluster not managed by Okteto it is not necessary to validate the externals // because they will not be deployed. if okteto.IsOkteto() { - control, err := ld.GetExternalControl(ld.K8sClientProvider, ld.TempKubeconfigFile) + control, err := ld.GetExternalControlForValidator(ld.K8sClientProvider) if err != nil { return err } diff --git a/pkg/cmd/build/logger.go b/pkg/cmd/build/logger.go index af587ac0c0b7..d284ce32a45e 100644 --- a/pkg/cmd/build/logger.go +++ b/pkg/cmd/build/logger.go @@ -31,7 +31,6 @@ const ( ) func deployDisplayer(ctx context.Context, ch chan *client.SolveStatus) error { - // TODO: import build timeout timeout := time.NewTicker(10 * time.Minute) defer timeout.Stop() @@ -46,6 +45,7 @@ func deployDisplayer(ctx context.Context, ch chan *client.SolveStatus) error { for { select { case <-ctx.Done(): + oktetoLog.StopSpinner() return ctx.Err() case <-timeout.C: case ss, ok := <-ch: @@ -60,6 +60,7 @@ func deployDisplayer(ctx context.Context, ch chan *client.SolveStatus) error { done = true } if done { + oktetoLog.StopSpinner() return nil } } diff --git a/pkg/model/manifest.go b/pkg/model/manifest.go index fc55b07a5a3a..232d8ea4056d 100644 --- a/pkg/model/manifest.go +++ b/pkg/model/manifest.go @@ -237,6 +237,13 @@ func NewDeployInfo() *DeployInfo { } } +// NewDeployInfo creates a deploy Info +func NewDestroyInfo() *DestroyInfo { + return &DestroyInfo{ + Commands: []DeployCommand{}, + } +} + func getManifestFromOktetoFile(cwd string) (*Manifest, error) { manifestPath, err := discovery.GetOktetoManifestPath(cwd) if err != nil { diff --git a/pkg/model/serializer.go b/pkg/model/serializer.go index 1d10a34c75ab..81dc5033c727 100644 --- a/pkg/model/serializer.go +++ b/pkg/model/serializer.go @@ -1078,6 +1078,61 @@ func isManifestFieldNotFound(err error) bool { return false } +func (d *DestroyInfo) UnmarshalYAML(unmarshal func(interface{}) error) error { + var commandsString []string + err := unmarshal(&commandsString) + if err == nil { + d.Commands = []DeployCommand{} + for _, cmdString := range commandsString { + d.Commands = append(d.Commands, DeployCommand{ + Name: cmdString, + Command: cmdString, + }) + } + return nil + } + var commands []DeployCommand + err = unmarshal(&commands) + if err == nil { + d.Commands = commands + return nil + } + type destroyInfoRaw DestroyInfo + var destroy destroyInfoRaw + err = unmarshal(&destroy) + if err != nil { + return err + } + + *d = DestroyInfo(destroy) + return nil +} + +func (d *DestroyInfo) MarshalYAML() (interface{}, error) { + isCommandList := true + for _, cmd := range d.Commands { + if cmd.Command != cmd.Name { + isCommandList = false + } + } + if isCommandList { + result := []string{} + for _, cmd := range d.Commands { + result = append(result, cmd.Command) + } + return result, nil + } + return d, nil +} + +func (m *Manifest) MarshalYAML() (interface{}, error) { + if m.Destroy == nil || len(m.Destroy.Commands) == 0 { + m.Destroy = nil + return m, nil + } + return m, nil +} + func (d *Dev) MarshalYAML() (interface{}, error) { type dev Dev // prevent recursion toMarshall := dev(*d) diff --git a/pkg/model/serializer_test.go b/pkg/model/serializer_test.go index c41f161f048b..39373cad3b78 100644 --- a/pkg/model/serializer_test.go +++ b/pkg/model/serializer_test.go @@ -1800,6 +1800,211 @@ devs: } } +func TestManifestMarshalling(t *testing.T) { + tests := []struct { + name string + manifest *Manifest + expected string + }{ + { + name: "destroy not empty", + manifest: &Manifest{ + Destroy: &DestroyInfo{ + Commands: []DeployCommand{ + { + Name: "hello", + Command: "hello", + }, + }, + }, + }, + expected: "destroy:\n- hello\n", + }, + { + name: "destroy empty", + manifest: &Manifest{}, + expected: "{}\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + marshalled, err := yaml.Marshal(tt.manifest) + if err != nil { + t.Fatal(err) + } + + if string(marshalled) != tt.expected { + t.Errorf("didn't marshal correctly. Actual %s, Expected %s", marshalled, tt.expected) + } + }) + } +} + +func TestDestroyInfoMarshalling(t *testing.T) { + tests := []struct { + name string + destroyInfo *DestroyInfo + expected string + }{ + { + name: "same-name-and-cmd", + destroyInfo: &DestroyInfo{Commands: []DeployCommand{ + { + Name: "okteto build", + Command: "okteto build", + }, + { + Name: "okteto deploy", + Command: "okteto deploy", + }, + }}, + expected: "- okteto build\n- okteto deploy\n", + }, + { + name: "full", + destroyInfo: &DestroyInfo{ + Image: "test", + Commands: []DeployCommand{ + { + Name: "build", + Command: "okteto build", + }, + { + Name: "deploy", + Command: "okteto deploy", + }, + }}, + expected: "image: test\ncommands:\n- name: build\n command: okteto build\n- name: deploy\n command: okteto deploy\n", + }, + { + name: "different-name-cmd", + destroyInfo: &DestroyInfo{Commands: []DeployCommand{ + { + Name: "build", + Command: "okteto build", + }, + { + Name: "deploy", + Command: "okteto deploy", + }, + }}, + expected: "commands:\n- name: build\n command: okteto build\n- name: deploy\n command: okteto deploy\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + marshalled, err := yaml.Marshal(tt.destroyInfo) + if err != nil { + t.Fatal(err) + } + + if string(marshalled) != tt.expected { + t.Errorf("didn't marshal correctly. Actual %s, Expected %s", marshalled, tt.expected) + } + }) + } +} + +func TestDestroyInfoUnmarshalling(t *testing.T) { + tests := []struct { + name string + input []byte + expected *DestroyInfo + isErrorExpected bool + }{ + { + name: "list of commands", + input: []byte(` +- okteto stack deploy`), + expected: &DestroyInfo{ + Commands: []DeployCommand{ + { + Name: "okteto stack deploy", + Command: "okteto stack deploy", + }, + }, + }, + }, + { + name: "list of commands extended", + input: []byte(` +- name: deploy stack + command: okteto stack deploy`), + expected: &DestroyInfo{ + Commands: []DeployCommand{ + { + Name: "deploy stack", + Command: "okteto stack deploy", + }, + }, + }, + }, + { + name: "commands", + input: []byte(`commands: +- okteto stack deploy`), + expected: &DestroyInfo{ + Commands: []DeployCommand{ + { + Name: "okteto stack deploy", + Command: "okteto stack deploy", + }, + }, + }, + }, + { + name: "compose with endpoints", + input: []byte(`compose: + manifest: path + endpoints: + - path: / + service: app + port: 80`), + expected: &DestroyInfo{ + Commands: []DeployCommand{}, + }, + isErrorExpected: true, + }, + { + name: "all together", + input: []byte(`commands: +- kubectl apply -f manifest.yml +compose: + manifest: ./docker-compose.yml + endpoints: + - path: / + service: frontend + port: 80 + - path: /api + service: api + port: 8080`), + expected: &DestroyInfo{ + Commands: []DeployCommand{}, + }, + isErrorExpected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := NewDestroyInfo() + + err := yaml.UnmarshalStrict(tt.input, &result) + if err != nil && !tt.isErrorExpected { + t.Fatalf("Not expecting error but got %s", err) + } else if tt.isErrorExpected && err == nil { + t.Fatal("Expected error but got none") + } + + if !assert.Equal(t, tt.expected, result) { + t.Fatal("Failed") + } + }) + } +} + func TestDeployInfoUnmarshalling(t *testing.T) { tests := []struct { name string