fabric8io / fabric8-maven-plugin

📢 This project is migrated to 👉 https://github.com/eclipse/jkube
Apache License 2.0
334 stars 202 forks source link

Plugin overwrite selector:matchLabels:app for Deployment #1326

Closed ilterpehlivan closed 5 years ago

ilterpehlivan commented 6 years ago

Description

Plugin overwrites my value below:

My resource under fabric8 directory:

spec:
  replicas: 1
  selector:
        matchLabels:
          **app: redis-ha**
          name: redis-ha-sentinel
  template:
    metadata:
      labels:
        app: redis-ha
        component: sentinel
        name: redis-ha-sentinel

Generated resource:

spec:
    replicas: 1
    revisionHistoryLimit: 2
    selector:
      matchLabels:
        **app: common**
        name: redis-ha-sentinel
        provider: fabric8

Info


* Kubernetes / OpenShift setup and version : Kubernetes 1.10 , minikube
* If it's a bug, how to reproduce :
* If it's a feature request, what is your use case :
* Sample project : *[GitHub Clone URL]*
stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

rhuss commented 6 years ago

This is for sure a bug that we should tackle.

devang-gaur commented 6 years ago

Can anyone try reproducing this issue please?

rohanKanojia commented 6 years ago

I tried reproducing this, and I'm able to reproduce. Here are the steps for reproducing:

  1. Go to samples/external-resources and change fmp version to desired one(in my case 4.0-SNAPSHOT).

  2. Remove profiles.yml from src/main/fabric8(somehow it causes build to fail), and change src/main/fabric8/deployment.yml to this:

src/main/fabric8:

spec:
  replicas: 1
  selector:
    matchLabels:
      app: redis-ha
      name: redis-ha-sentinel
  template:
    spec:
      volumes:
        - name: config
          gitRepo:
            repository: 'https://github.com/jstrachan/sample-springboot-config.git'
            revision: 667ee4db6bc842b127825351e5c9bae5a4fb2147
            directory: .
      containers:
        - volumeMounts:
            - name: config
              mountPath: /app/config
          env:
            - name: KUBERNETES_NAMESPACE
              valueFrom:
                fieldRef:
                  apiVersion: v1
                  fieldPath: metadata.namespace
      serviceAccount: ribbon
  1. Run mvn fabric8:resource and check contents of generated resources, in my case app matchLabel was overridden by fmp:

target/classes/META-INF/fabric8/kubernetes/fabric8-maven-sample-external-resources-deployment.yml:

spec:
  replicas: 1
  revisionHistoryLimit: 2
  selector:
    matchLabels:
      app: fabric8-maven-sample-external-resources
      name: redis-ha-sentinel
      provider: fabric8
      group: io.fabric8
  template:
    metadata:
      annotations:
        fabric8.io/git-commit: e761c400b8e2dfaaf076bf1d482f39ef7ddda51a
        fabric8.io/git-branch: master
        fabric8.io/scm-tag: HEAD
        fabric8.io/scm-url: https://github.com/spring-projects/spring-boot/spring-boot-starter-parent/fabric8-maven-sample-external-resources
      labels:
        app: fabric8-maven-sample-external-resources
        provider: fabric8
        version: 4.0-SNAPSHOT
        group: io.fabric8
    spec:
      containers:
      - env:
        - name: KUBERNETES_NAMESPACE
          valueFrom:
            fieldRef:
              fieldPath: metadata.namespace
        image: fabric8-maven-sample-external-resources:latest
        imagePullPolicy: IfNotPresent
        livenessProbe:
          httpGet:
            path: /health
            port: 8080
            scheme: HTTP
          initialDelaySeconds: 180
        name: spring-boot
        ports:
        - containerPort: 8080
          name: http
          protocol: TCP
        - containerPort: 9779
          name: prometheus
          protocol: TCP
        - containerPort: 8778
          name: jolokia
          protocol: TCP
        readinessProbe:
          httpGet:
            path: /health
            port: 8080
            scheme: HTTP
          initialDelaySeconds: 10
        securityContext:
          privileged: false
        volumeMounts:
        - mountPath: /app/config
          name: config
      serviceAccount: ribbon
      volumes:
      - gitRepo:
          directory: "."
          repository: https://github.com/jstrachan/sample-springboot-config.git
          revision: 667ee4db6bc842b127825351e5c9bae5a4fb2147
        name: config
devang-gaur commented 6 years ago

The bug is due to the override of Map entries here in the DeploymentSpecBuilderVisitor

https://github.com/fabric8io/fabric8-maven-plugin/blob/master/plugin/src/main/java/io/fabric8/maven/plugin/enricher/SelectorVisitor.java#L101 .

and the fix is to replace the line 101 with

MapUtil.mergeIfAbsent(selector.getMatchLabels(), selectorMatchLabels);

(^ refer for the function to https://github.com/fabric8io/fabric8-maven-plugin/blob/master/core/src/main/java/io/fabric8/maven/core/util/MapUtil.java#L41 ) .

However, if we make the fix only here, then this behaviour won't be consistent with the behaviour of other SpecBuilderVisitors in the file. They all use the pattern.

selector.getMatchLabels().putAll(selectorMatchLabels)

and this is problematic as the current Map entries get override everytime . We need to change this line to

MapUtil.mergeIfAbsent(selector.getMatchLabels(), selectorMatchLabels);

at every occurence here in the file.

This is what I understood of the scenario. Let me know what should be done here and I'll send the PR.

@rhuss @lordofthejars @ro14nd @rohanKanojia

lordofthejars commented 6 years ago

@dev-gaur I faced similar problem in case of overriding environment variables between configuration and fragments. So I created this PR which might be a solution. (https://github.com/fabric8io/fabric8-maven-plugin/pull/1434/files) which basically implements logic in BaseEnricher and implementators are free to call it or not.

This might be the first step, then we should decide if it is always called or let implementators decide.