okteto exec to work the same as kubectl exec

[jLopezbarb]

DEV-327

Proposed changes

In this PR we've addressed that okteto exec was behaving different to kubectl exec. Also we didn't have any test for that command, which now we can have.

• Use the same pre-processing as kubernetes for kubectl exec and okteto exec. We still maintain the selector in case the user has more than one dev environment and doesn´t add it to the arguments
• Add unit tests for the whole command

How to validate

1. Run okteto up on any okteto configured repository
2. Run okteto exec name -- echo test and check that is working
3. Run okteto exec name -- "echo test" and observe an error

CLI Quality Reminders 🔧

For both authors and reviewers:

• Scrutinize for potential regressions
• Ensure key automated tests are in place
• Build the CLI and test using the validation steps
• Assess Developer Experience impact (log messages, performances, etc)
• If too broad, consider breaking into smaller PRs
• Adhere to our code style and code review guidelines

[codecov]

Codecov Report

Attention: Patch coverage is 51.49254% with 130 lines in your changes are missing coverage. Please review.

Project coverage is 43.35%. Comparing base (19eb936) to head (f1e049a).
Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4289      +/-   ##
==========================================
+ Coverage   43.10%   43.35%   +0.24%     
==========================================
  Files         371      375       +4     
  Lines       30021    30138     +117     
==========================================
+ Hits        12940    13065     +125     
+ Misses      15954    15943      -11     
- Partials     1127     1130       +3     

[andreafalzetti]

Great work 👏 Code changes LGTM. I've tested different scenarios and everything worked for me besides the following:

1. Using: https://github.com/andreafalzetti/hybrid-test
2. okteto up
3. once the up session is up & running in another terminal run: okteto exec hybridapp -l debug -- time
4. notice how the command "time" is not executed. I've tried with touch file.txt too and it seems like it's a problem of not executing the command instead of a problem of "output".

Update

As we noticed offline, it only seems to be related to time. Other commands execute corretly.

[ifbyol]

There is nothing blocker but some comments would be good to address

[ifbyol]

I have been testing it and I found small things to improve or thing that have changed.

1. When you don't specify a dev env where to execute the command, before, we were displaying the selector with just the dev environments in dev mode. Now, we display all the devs (in that screen, the terminal in the bottom is current CLI version and the other one is the version of this branch). If you select a dev which is not in dev mode, it fails (which is fine), but I think we should display only the services that are in dev mode

2. When a command fails the execution, we are displaying the error message (the red one) in the same line that previous errors, Could we just add a new line to show the red message in a new line? This is not new, but if it is something easy to fix

3. When I execute okteto exec without specifying a service (and there are several services) and without specifying a command, I have to select the dev env first, to then, fail with an error indicating that a command is required. Could we do the validation first instead of making the user to select a dev to then, fail?

[ifbyol]

Do we have some e2e test for the exec command?

[jLopezbarb]

Do we have some e2e test for the exec command?

Not a dedicated one right now. We're checking if the stignore is created in the remote by using okteto exec on all okteto ups, but we don't have a dedicated one. I'll create two, one for autocreate and another one for the normal flow

[ifbyol]

The error message when I try to execute a command for a non-existing dev container has changed with a less meaningful error and we are missing the hint. Is that expected?

With these changes

Without these changes

We should not change that unless it is strictly necessary. As it was before it is giving better DX and giving hints to the user on why it failed and how it can be solved

[ifbyol]

Something similar happens when you don't specify a dev, but there are no devs on dev mode

With these changes

Without these changes

Specially, this error Failed to set dev from manifest is not meaningful and can be misleading for users