feat: multistages now respect dependencies without building unnecessary stages

[JordanGoasdoue]

Fixes #815

Description

⚠️ Started From this branch JordanGoasdoue:can-now-resolve-args-from-stage in this PR : #1160 to be able to have the args resolved correctly. The diff about this PR without taking into account #1160 is linked here : 2b45013
Multistage by kaniko doesn't have unnecessary builded stages anymore

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes unit tests
• Adds integration tests if needed.

See the contribution guide for more details.

Reviewer Notes

• The code flow looks good.
• Unit tests and or integration tests added.

Release Notes

Kaniko now respect multistage dependency tree without building unnecessary intermediates stages

Examples of user facing changes:
- kaniko adds a new flag `--registry-repo` to override registry

[JordanGoasdoue]

As this following dockerfile test from here pkg/executor/build_test.go can only work if it builds all the stages :

FROM debian as stage1
FROM ubuntu as stage2
RUN foo
COPY --from=stage1 /foo /bar
FROM alpine
COPY --from=stage1 /baz /bat

I have edited it to

FROM ubuntu as stage1
RUN foo
FROM alpine
COPY --from=stage1 /foo /bar
COPY --from=stage1 /baz /bat

because stage2 is never called on the case we are building without unnecessary stages.
Would be better I guess to add a parameter on the cli which would do this optimized build by calling the new function getUsedStaged only when the cli parameter is activated in order to not break anything, taking this dockerfile as an example. What do you think guys?

[JordanGoasdoue]

@tejal29 , the PR is now working as expected
If you are planning to have on kaniko this feature which doesn't build unusued stages, do you want a specific cli parameter to activate it as a new mode ?

[tejal29]

This is a great feature! It would be great if you can use a cli flag to expose this feature and turn it off by default.
--build-used-stages=false.

In future, we can make this default behavior.

[JordanGoasdoue]

@tejal29, perfect, will do it.
But first, do I need to rebase both my PRs (this one and #1160) on the branch from this PR: #1190 ?

[dani29]

Hi Jordan, I rebased #1190 to resolve most of the conflicts with your previous PR - can you take a quick look and rebase your PR on it? Thanks!

[JordanGoasdoue]

Done @dani29
Cheers

[dani29]

Thanks for the rebase!
It's a bit nitpicky, but I just realized that "build-used-stages" might be confusing for the user, as in both cases (t/f) you do build the used stages... it's the unused/unnecessary stages that you ignore.

Do you think it'd make more sense to call it --skip-unused-stages/--ignore-unused-stages or something like that?

[JordanGoasdoue]

You're welcome.
True, I prefer even more --skip-unused-stages, it's clearer as you said.. I'm doing the change right now.

[JordanGoasdoue]

I've had some concerns about the function skipUnusedStages, especially with the targetIndex previously passed by value and retrieve by value after, it's why I've done some change on it.

Now targetIndex is passed by reference, and directly updated on the function if it needs to. No needs to retrieve it by value then.

I've also added some tests about the targetIndex, to compare It between the first one calculated before skipUnusedStages is called : expectedTargetIndexBeforeSkip, and the one updated after skipUnusedStages is called : expectedTargetIndexAfterSkip

[tejal29]

Thanks @JordanGoasdoue I will take a look and get it in today.