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.

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

Unwanted container creation in deploymentConfig.yamls due to initContainer #1316

Closed
fscdf opened this issue Feb 24, 2022 · 6 comments · Fixed by #1969
Closed

Unwanted container creation in deploymentConfig.yamls due to initContainer #1316

fscdf opened this issue Feb 24, 2022 · 6 comments · Fixed by #1969
Assignees
Milestone

Comments

@fscdf
Copy link

fscdf commented Feb 24, 2022

Description

I want to create two docker images in the pom, then run one in an initContainer and one in a normal container. All generated deploymentConfig yaml's contain, however, an additional unwanted container for the second image in the pom.

Info

The relevant portion of my pom.xml:

                    <plugin>
                        <groupId>org.eclipse.jkube</groupId>
                        <artifactId>openshift-maven-plugin</artifactId>
                        <version>1.5.1</version>
                        <executions>
                            <execution>
                                <goals>
                                    <goal>resource</goal>
                                    <goal>apply</goal>
                                    <goal>build</goal>
                                </goals>
                            </execution>
                        </executions>
                        <configuration>
                            <resources>
                                <openshiftBuildConfig>
                                    <requests>
                                        <cpu>1</cpu>
                                        <memory>1Gi</memory>
                                    </requests>
                                    <limits>
                                        <cpu>3</cpu>
                                        <memory>3Gi</memory>
                                    </limits>
                                </openshiftBuildConfig>
                            </resources>
                            <images>
                                <image>
                                    <name>init-image</name>
                                    <alias>init-image-alias</alias>
                                    <build>
                                        <assembly>
                                            <inline>
                                                <files>
                                                    <file>
                                                        <source>
                                                            ${project.basedir}/example_file.txt
                                                        </source>
                                                        <outputDirectory>.</outputDirectory>
                                                    </file>
                                                </files>
                                            </inline>
                                            <name>folder</name>
                                        </assembly>
                                        <from>
                                            some-source:latest
                                        </from>
                                    </build>
                                </image>
                                <image>
                                    <name>backend</name>
                                    <alias>backend-alias</alias>
                                    <build>
                                        <assembly>
                                            <inline>
                                                <files>
                                                    <file>
                                                        <source>
                                                            ${project.basedir}/example_backend_file.txt
                                                        </source>
                                                        <outputDirectory>.</outputDirectory>
                                                    </file>
                                                </files>
                                            </inline>
                                            <name>target_folder</name>
                                        </assembly>
                                        <from>
                                            some-backend-source:latest
                                        </from>
                                    </build>
                                </image>
                            </images>
                        </configuration>
                    </plugin>

The initContainer and container are defined as follows.

spec:
      initContainers:
        name: init-image
        image: init-image:latest
        imagePullPolicy: Always
        resources:
          limits:
            cpu: 1
            memory: 1Gi
          requests:
            cpu: 3
            memory: 3Gi
      containers:
        image: backend:latest
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 1
        readinessProbe:
          failureThreshold: 3
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 1
        name: backend
        resources:
            limits:
              cpu: 1
              memory: 1Gi
            requests:
              cpu: 3
              memory: 3Gi
      dnsPolicy: ClusterFirst
      restartPolicy: Always
      schedulerName: default-scheduler
      securityContext: {}
      terminationGracePeriodSeconds: 30

In the target/ folder, all generated deploymentConfigs have this container, which shouldn't be there:

- env:
        - name: KUBERNETES_NAMESPACE
          valueFrom:
            fieldRef:
              fieldPath: metadata.namespace
        image: backend:latest
        imagePullPolicy: IfNotPresent
        name: backend-alias
        securityContext:
          privileged: false

It is important to note, that this unwanted container seems to be a copy of the backend container. This is due to the backend image coming second in the pom. If the order is reversed, the above container will have the init-image.

If, instead of an initContainer and a container, I define two containers in the deployment.yaml, everything works fine.

Not sure if this is a bug or flawed setup.

@sunix
Copy link
Member

sunix commented Nov 23, 2022

TODO: try to reproduce

@rohanKanojia rohanKanojia self-assigned this Dec 8, 2022
@rohanKanojia
Copy link
Member

@fscdf : Thanks for reporting, I can reproduce your issue. From what I see you want to build two images:

  • one that would be consumed by initContainer
  • one that would be main container

At the moment, <images> only seem to map to .spec.template.spec.containers[] field in the generated controller (Deployment, DeploymentConfig etc.)

I think you can work around this issue by separating plugin configuration for building initContainer's image and main container's image. This would take care of building image separately from main application container image. For adding initContainer into DeploymentConfig, you can add a fragment with initContainer details:

spec:
  template:
    spec:
      initContainers:
      - name: init-image
        image: init-image:latest
        imagePullPolicy: Always
        resources:
          limits:
            cpu: 1
            memory: 1Gi
          requests:
            cpu: 3
            memory: 3Gi

I've created a demo project, you can see configuration here

$ mvn oc:build -Pinitcontainer-build # Build InitContainer image
$ mvn oc:build oc:resource oc:apply # Build application

However, this would add an extra step for building initContainers image into cluster. It doesn't look good.

I think we need to discuss and add functionality to support initContainers in generated controllers. Or maybe add filters so that some images can only be considered for certain phases (build only, resource only, etc.)

@sunix
Copy link
Member

sunix commented Dec 9, 2022

To me, it should be a new feature: be able to tell that a container should be an initContainer

@rohanKanojia
Copy link
Member

I propose adding a new configuration option to our resource configuration:

  <configuration>
     <resources>
         <initContainers>
            <initContainer>
                <name>init-image</name>
                <image>init-image:latest</image>
                <imagePullPolicy>Always</imagePullPolicy>
                ...
            </initContainer>
         </initContainers>
     </resources>
  </configuration>

ContainerHandler would skip ImageConfiguration's which are present in .initContainer config.

InitContainerHandler should be modified to generate InitContainers from XML configuration.

@manusa
Copy link
Member

manusa commented Dec 12, 2022

Current behavior

The VolumePermissionEnricher and InitContainerHandler are currently used to change permissions of mounted directories by running a chmod command when the container is initialized using an initContainer. The behavior for this Enricher should be preserved and guaranteed.

Desired behavior

  • Users can configure initContainers entries using XML/DSL -> Unwanted container creation in deploymentConfig.yamls due to initContainer #1316 (comment)
  • initContainer XML/DSL configuration provides the following settings
    • Name (required)
    • Image name (required)
    • Image Pull Policy
    • command
  • Init containers can optionally use JKube generated images
    • If the image used matches one of those generated by JKube, this image won't be used to create an additional container

This would allow to easily add a setting such as:

<initContainer>
  <name>test</name>
  <imageName>busybox</imageName>
  <command>
    <arg>sh</arg>
    <arg>-c</arg>
    <arg>until nslookup mydb.$(cat /var/run/secrets/kubernetes.io/serviceaccount/namespace).svc.cluster.local; do echo waiting for mydb; sleep 2; done</arg>
  </command>
</initContainer>

ResourceConfig refactorings

Since we are adding more and more configuration options to Controllers, the ResourceConfig class and the user experience when dealing with its XML/DSL version is getting worse.

It would be good to group all of the Controller-related settings in a Controller class and provide a top-level ResourceConfig.controller(s) (Collection) field where users could provide these settings in a nested/grouped fashion

In order to be able to provide support for the legacy settings, we would mark the current fields as deprecated and provide them within the Controller class too.
Fields defined in the Controller class would take precedence over those defined in the ResourceConfig class.

Multiple controllers

We need to decide if the API should support multiple controller definitions, or maybe just provide support for both, the singular and the plural and aggregate both.

<configuration>
  <resources>
     <!-- We should support both configurations -->
     <controller><!-- ... --></controller>
     <controllers>
       <controller><!-- ... --></controller>
     </controllers>
  </resources>
</configuration>

References

@rohanKanojia
Copy link
Member

We need to decide if the API should support multiple controller definitions, or maybe just provide support for both, the singular and the plural and aggregate both.

I'm a little confused about how we should approach providing multiple controller support or whether we should even do it. Typically our plugin is installed per project and it generates a single controller+service only for that specific project.

If we introduce multiple controllers, I'm a little confused about how we should handle these scenarios:

  • In goals like k8s:log, k8s:debug, k8s:watch how would we decide which deployed controller should be used?
  • If there is a fragment provided by user and multiple controllers defined in resource configuration. How would we know which controller should be merged with the fragment?
  • If user provides DefaultControllerEnricher enricher configuration and controller configuration, how should we merge these both?
  • Would JKube also create Service equivalents of these additionally added controllers?

If we switch from single controller generation to multiple controller generation, we would need to modify the behavior of these enrichers:

  • ControllerViaPluginConfigurationEnricher
  • DefaultControllerEnricher
  • DefaultServiceEnricher
  • ImageEnricher
  • NameEnricher
  • VolumePermissionEnricher

rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 15, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 16, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 19, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 19, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 20, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 20, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 20, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 20, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 20, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 20, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 21, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 21, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 21, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 21, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 21, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 21, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 21, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 22, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 22, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 22, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 22, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Dec 23, 2022
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Jan 3, 2023
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Jan 30, 2023
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Feb 7, 2023
…urce configuration (eclipse-jkube#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@manusa manusa added this to the 1.11.0 milestone Feb 8, 2023
manusa pushed a commit that referenced this issue Feb 8, 2023
…urce configuration (#1316)

+ Deprecate all controller specific configuration options in
  ResourceConfig. Move all controller specific options to nested controller configuration.
+ Add controller(s) configuration option in ResourceConfig
+ Add initContainres configuraiton option in Controller configuration

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants