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.

Sign up for GitHub

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

Jump to bottom

Fix okteto endpoints namespace detection from okteto.yml #4186

Merged
merged 8 commits into from
Feb 23, 2024
Merged

Fix okteto endpoints namespace detection from okteto.yml #4186

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a4e7a04
fix(endpoints): detect namespace from manifest and check for manifest…
andreafalzetti Feb 20, 2024
bcf3330
update error msg
andreafalzetti Feb 20, 2024
3a313a3
fix linter
andreafalzetti Feb 20, 2024
f771f39
fix: endpoints output
andreafalzetti Feb 20, 2024
f512832
revert unecessary change
andreafalzetti Feb 20, 2024
04d2425
fix: header
andreafalzetti Feb 21, 2024
e0b7d6b
change default options not pointer
andreafalzetti Feb 21, 2024
231827a
fix: handle case when namespace is provided in both places
andreafalzetti Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 5 additions & 7 deletions cmd/context/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func LoadStackWithContext(ctx context.Context, name, namespace string, stackPath
}

// LoadContextFromPath initializes the okteto context taking into account command flags and manifest namespace/context fields
func LoadContextFromPath(ctx context.Context, namespace, k8sContext, path string) error {
func LoadContextFromPath(ctx context.Context, namespace, k8sContext, path string, defaultCtxOpts Options) error {
ctxResource, err := getCtxResource(path)
if err != nil {
return err
Expand All @@ -238,11 +238,9 @@ func LoadContextFromPath(ctx context.Context, namespace, k8sContext, path string
return err
}

ctxOptions := &Options{
Context: ctxResource.Context,
Namespace: ctxResource.Namespace,
Show: true,
}
ctxOptions := defaultCtxOpts
ctxOptions.Context = ctxResource.Context
ctxOptions.Namespace = ctxResource.Namespace

return NewContextCommand().Run(ctx, ctxOptions)
return NewContextCommand().Run(ctx, &ctxOptions)
}
2 changes: 1 addition & 1 deletion cmd/deploy/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func Deploy(ctx context.Context, at analyticsTrackerInterface, ioCtrl *io.Contro
}

// Loads, updates and uses the context from path. If not found, it creates and uses a new context
if err := contextCMD.LoadContextFromPath(ctx, options.Namespace, options.K8sContext, options.ManifestPath); err != nil {
if err := contextCMD.LoadContextFromPath(ctx, options.Namespace, options.K8sContext, options.ManifestPath, contextCMD.Options{Show: true}); err != nil {
if err.Error() == fmt.Errorf(oktetoErrors.ErrNotLogged, okteto.CloudURL).Error() {
return err
}
Expand Down
37 changes: 13 additions & 24 deletions cmd/deploy/endpoints.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"strings"

contextCMD "github.com/okteto/okteto/cmd/context"
"github.com/okteto/okteto/cmd/utils"
"github.com/okteto/okteto/pkg/devenvironment"
"github.com/okteto/okteto/pkg/endpoints"
oktetoErrors "github.com/okteto/okteto/pkg/errors"
Expand All @@ -32,6 +31,7 @@ import (
"github.com/okteto/okteto/pkg/log/io"
"github.com/okteto/okteto/pkg/model"
"github.com/okteto/okteto/pkg/okteto"
"github.com/spf13/afero"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -83,6 +83,7 @@ func NewEndpointGetter(k8sLogger *io.K8sLogger) (EndpointGetter, error) {
// Endpoints deploys the okteto manifest
func Endpoints(ctx context.Context, k8sLogger *io.K8sLogger) *cobra.Command {
options := &EndpointsOptions{}
fs := afero.NewOsFs()
cmd := &cobra.Command{
Use: "endpoints",
Short: "Show endpoints for an environment",
Expand All @@ -93,32 +94,20 @@ func Endpoints(ctx context.Context, k8sLogger *io.K8sLogger) *cobra.Command {
return err
}
options.ManifestPath = model.GetManifestPathFromWorkdir(options.ManifestPath, workdir)
}

ctxResource, err := utils.LoadManifestContext(options.ManifestPath)
if err != nil {
if oktetoErrors.IsNotExist(err) {
ctxResource = &model.ContextResource{}
// check whether the manifest file provided by -f exists or not
if _, err := fs.Stat(options.ManifestPath); err != nil {
return oktetoErrors.UserError{
E: fmt.Errorf("the okteto manifest file '%s' does not exist", options.ManifestPath),
Hint: "Check the path to the okteto manifest file",
}
}
}

if err := ctxResource.UpdateNamespace(options.Namespace); err != nil {
return err
}

if err := ctxResource.UpdateContext(options.K8sContext); err != nil {
return err
}

ctxOptions := &contextCMD.Options{
Context: ctxResource.Context,
Namespace: ctxResource.Namespace,
}
if options.Output == "" {
ctxOptions.Show = true
}
if err := contextCMD.NewContextCommand().Run(ctx, ctxOptions); err != nil {
return err
// Loads, updates and uses the context from path. If not found, it creates and uses a new context
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same logic used in okteto deploy

if err := contextCMD.LoadContextFromPath(ctx, options.Namespace, options.K8sContext, options.ManifestPath, contextCMD.Options{Show: options.Output == ""}); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Why for the endpoints command the Show value would depend on the options.Output value but for the deploy and destroy command it is always true. What is special about the endpoints command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the okteto endpoints command can be used as follows:

okteto endpoints --output=json | jq XYZ

or

okteto endpoints --output=md > README.md

In those scenarios we cannot output the plaintext header or it would break those scenarios. For build and deploy we don't have such use-cases, or at least I am not aware or them, so the Show value - only for endpoints depend on its --output flag.

Worth mentioning that for "plaintext" in most commands of the CLI we use empty string. So by default an empty --output flag means "plaintext". So the check is a bit odd. I hope that with 3.0 we can add a third allowed value for --output plaintext or --output text to make this more explicit.

Copy link
Member

Choose a reason for hiding this comment

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

For deploy and destroy we have the global flag --log-output which could be also json (I was confusing the flag output with that global flag). I was testing deploy and destroy, and when the --log-output flag is set, we are not displaying the context information even if we set Show: true in the context.

I see it working fine but I think this is confusing for the user. If I execute okteto endpoints --log-output=json it doesn't print anything in the output

if err := contextCMD.NewContextCommand().Run(ctx, &contextCMD.Options{Namespace: options.Namespace, Show: false}); err != nil {
return err
}
}

eg, err := NewEndpointGetter(k8sLogger)
Expand Down
2 changes: 1 addition & 1 deletion cmd/destroy/destroy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func Destroy(ctx context.Context, at analyticsTrackerInterface, ioCtrl *io.Contr
}
options.ManifestPath = uptManifestPath
}
if err := contextCMD.LoadContextFromPath(ctx, options.Namespace, options.K8sContext, options.ManifestPath); err != nil {
if err := contextCMD.LoadContextFromPath(ctx, options.Namespace, options.K8sContext, options.ManifestPath, contextCMD.Options{Show: true}); err != nil {
if err.Error() == fmt.Errorf(oktetoErrors.ErrNotLogged, okteto.CloudURL).Error() {
return err
}
Expand Down