Use .oktetoignore in deploy and destroy remote

[maroshii]

We now inject rules for destroy and deploy remote a we do for test. This is merged with the contents of .oktetodeployignore file.

Updated docs as part of this: okteto/docs#743

Validation

Create the following folders:

mkdir report-never && echo never > report-never/never
mkdir report-deploy && echo deploy > report-deploy/deploy
mkdir report-destroy && echo destroy > report-destroy/destroy

Using the following .oktetoignore file:

report-never

[deploy]
report-destroy

[destroy]
report-deploy

with an ls in the deploy and destroy sections:

deploy:
  - ls

destroy:
  - ls

For deploy we should only get report-deploy and for destroy we should only get report-destroy

[codecov]

Codecov Report

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

Project coverage is 43.10%. Comparing base (c09877c) to head (13799ee).
Report is 4 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4316      +/-   ##
==========================================
- Coverage   43.10%   43.10%   -0.01%     
==========================================
  Files         371      371              
  Lines       30021    30042      +21     
==========================================
+ Hits        12940    12949       +9     
- Misses      15954    15960       +6     
- Partials     1127     1133       +6     

[ifbyol]

If both files are defined, we are merging both, which I think it is misleading. If the new file is defined, it should mean that users are migrated to new file, so I don't think we should keep taking into account .oktetodeployignore.

WDYT @pchico83 @codyjlandstrom ?

[ifbyol]

I think this warning is not located in the right place. We are adding the warning if .oktetodeployignore file doesn't exist in the working directory instead of showing it if it exists

As you can see if the screenshot, there is a .oktetodeployignore file but the warning is not present. If I run the command without that file, the warning is displayed

[ifbyol]

Does this work on windows? (referring to /%s)

[maroshii]

I assume that's what Clean is for. I simply moved this function to be a global in this package (it was previously a member of runner).

From Clean docs:

On Windows, Clean does not modify the volume name other than to replace occurrences of "/" with \.
For example, Clean("//host/share/../x") returns \\host\share\x.

[pchico83]

If both files are defined, we are merging both, which I think it is misleading. If the new file is defined, it should mean that users are migrated to new file, so I don't think we should keep taking into account .oktetodeployignore.

WDYT @pchico83 @codyjlandstrom ?

Both options sound good to me. But for simplicity, I would also go with one file or the other, without merging tham

[andreafalzetti]

Code changes LGTM. I've also tested the scenario described and it works both on Mac and Windows.

As already mentioned the warning for .oktetodeployignore appears when I don't have it, and it should be fixed before merging.

About the "merge" vs "override" I think both options are good, I defer to Product the final call, however I would go for now with the current implementation as it's good, and we can just drop the support for the old format in the future.

[maroshii]

I think what we have is ok (merging) until we fully deprecate the old file. User could be using .oktetoignore only for okteto test and not for deploy in which case no rules would be included.

[pchico83]

I think what we have is ok (merging) until we fully deprecate the old file. User could be using .oktetoignore only for okteto test and not for deploy in which case no rules would be included.

@maroshii SGTM