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 5 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
13 changes: 8 additions & 5 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 {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need a pointer for defaultCtxOpts? I see we always pass a value. We could just pass the value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no need to be pointer actually, the code is much better with a value type, changed in e0b7d6b

thanks!

ctxResource, err := getCtxResource(path)
if err != nil {
return err
Expand All @@ -238,11 +238,14 @@ func LoadContextFromPath(ctx context.Context, namespace, k8sContext, path string
return err
}

ctxOptions := &Options{
Context: ctxResource.Context,
Namespace: ctxResource.Namespace,
Show: true,
var ctxOptions *Options

if defaultCtxOpts != nil {
ctxOptions = defaultCtxOpts
}

ctxOptions.Context = ctxResource.Context
ctxOptions.Namespace = ctxResource.Namespace

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: false}); err != nil {
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