Issue#968 - Multi Container Support with Resource Fragments

[kameshsampath]

• added config to spring boot generator
• ability to parse multiple yaml docs with or without profiles

[codecov]

Codecov Report

Merging #969 into master will decrease coverage by 0.07%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master     #969      +/-   ##
============================================
- Coverage     22.51%   22.44%   -0.08%     
  Complexity      590      590              
============================================
  Files           166      166              
  Lines          8757     8786      +29     
  Branches       1531     1541      +10     
============================================
  Hits           1972     1972              
- Misses         6570     6599      +29     
  Partials        215      215

[nicolaferraro]

Thanks for the PR @kameshsampath. You've hit a general problem with f-m-p that is how to handle enrichment properly when you have multiple containers/controllers.

I think a better approach for handling resource fragments is the one I've highlighted in the comments. But since sidecars are something that will be used a lot in the future (I think that the plugin itself will generate sidecars using enrichers in the near future), I'd like to know what @rhuss thinks about that. Also if we can rely on this concept of "main project's container".

[nicolaferraro]

Here you're correlating over the image name. This works with sidecars, but it may have problems with pods having multiple containers from the same base image (with different startup options).

I think a better approach would be to correlate over the container name, i.e.

• If a fragment has a name, and the name is different the one that would be used for the current project, then add another container
• If the fragment does not declare a name, or the name matches the one that would be used for the current project, then use that as container for the project

[kameshsampath]

@nicolaferraro - do you mean this

...
for (Container defaultContainer : defaultContainers) {
                    Container container;
                    boolean isNewContainerAdded = false;
                    if (idx < containers.size()) {
                        container = containers.get(idx);
                    } else {
                        isNewContainerAdded = true;
                        container = new Container();
                        containers.add(container);
                    }
                    String containerName = container.getName();
                    String defaultContainerName = defaultContainer.getName() == null?defaultName:defaultContainer.getName();
                    **if (!defaultContainerName.equalsIgnoreCase(containerName) && !isNewContainerAdded)** {
                        container = new Container();
                        containers.add(container);
                    }
                    mergeSimpleFields(container, defaultContainer);
 ....

[nicolaferraro]

Something like that @kameshsampath, but I see that code adds another container if the fragment does not declare a name (that is the standard case when adding fragments). I'd add another container only if the one declared in the fragments uses a name that is different from the default container name for the project (e.g. it's called proxy). Please, also fix the dependencies of the 'it' tests as they refer snapshot artifacts.

[kameshsampath]

@nicolaferraro - am not getting what you mean " I see that code adds another container if the fragment does not declare a name" - can you please point to the line# in the code for it ?

[nicolaferraro]

We can decide to enrich all containers here, or we can rely here on the concept of "project's main container", or list the containers that should be enriched using a configuration option (also a global one if this enricher is not the only one that should be scoped to a specific container). I'd not go for "first" / "last" as it relies too much on what the current implementation does but it may change in the future (with plugin updates or with "fragments" added in other ways e.g. #941).

[kameshsampath]

@nicolaferraro i agree with you on the first/last/all, how does something like this

<config>
  <probes>
       <container-name-1/>
       <container-name-2/>
 </probes>
</config> 

when the probes is empty then we assume "all" and add probes to all containers

[nicolaferraro]

I'd keep the configuration inside the enricher config, but a list of container names can be used (I don't know if the configuration utils support lists as you have declared them in the example, there should be other cases of list configs). About the default when no configuration is given: both 'all containers' or 'just the one having the default name' are reasonable in my opinion.

[christophetd]

Any plans to continue working on this PR @kameshsampath? Otherwise, would you like some help on that?

[kameshsampath]

@christophetd adding @hrishin to this. he is one of the maintainers right now.

[rohanKanojia]

There is a PR open already for multicontainer support based on discussions of Kamesh and Nicola(which is properly rebased with master). I'm closing this one.