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

Add env_file + ConfigMaps feature to Kompose #799

Merged
merged 2 commits into from
Oct 10, 2017

Conversation

cdrage
Copy link
Member

@cdrage cdrage commented Aug 25, 2017

When using env_file with Docker Compose, a ConfigMap will be generated.

For example:

▶ ./kompose convert -f
script/test/fixtures/configmaps/docker-compose.yml
INFO Kubernetes file "redis-service.yaml" created
INFO Kubernetes file "redis-deployment.yaml" created
INFO Kubernetes file "foo-env-configmap.yaml" created
INFO Kubernetes file "bar-env-configmap.yaml" created

File:

version: '3'

services:
  redis:
    image: 'bitnami/redis:latest'
    environment:
      - ALLOW_EMPTY_PASSWORD=no
    # Env file will override environment / warn!
    env_file:
      - "foo.env"
      - bar.env
    labels:
      kompose.service.type: nodeport
    ports:
      - '6379:6379'

To:

apiVersion: v1
data:
  ALLOW_EMPTY_PASSWORD: "yes"
kind: ConfigMap
metadata:
  creationTimestamp: null
  name: foo-env
...
      - env:
        - name: ALLOW_EMPTY_PASSWORD
          valueFrom:
            configMapKeyRef:
              key: ALLOW_EMPTY_PASSWORD
              name: foo-env

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 25, 2017
@kompose-bot
Copy link
Collaborator

@cdrage, thank you for the pull request! We'll request some people to review your PR. @containscafeine and @procrypt, please review this.

@kompose-bot kompose-bot requested review from concaf and procrypt August 25, 2017 19:52
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 25, 2017
@cdrage
Copy link
Member Author

cdrage commented Aug 25, 2017

TODO:

  • Add tests
  • Remove conflicts (kobject.go)

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 28, 2017
@cdrage cdrage force-pushed the add-configmap branch 2 times, most recently from d070dde to 15b2de9 Compare August 28, 2017 15:22
@cdrage
Copy link
Member Author

cdrage commented Aug 28, 2017

Ready for review @ashetty1 @containscafeine @kadel @surajnarwade @surajssd

Copy link
Contributor

@concaf concaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docker Compose file -

version: '3'
  
services:
  redis:
    image: 'bitnami/redis:latest'
    environment:
      - ALLOW_EMPTY_PASSWORD=no
    # Env file will override environment / warn!
    env_file:
      - ./bar.env
    labels:
      kompose.service.type: nodeport
    ports:
      - '6379:6379'

Generated ConfigMap -

- apiVersion: v1
  data:
    BAR: FOO
    FOO: BAR
  kind: ConfigMap
  metadata:
    creationTimestamp: null
    name: -/bar-env
kind: List
metadata: {}

Upon doing kompose up -

$ kompose up -f script/test/fixtures/configmap/docker-compose.yaml 
INFO We are going to create Kubernetes Deployments, Services and PersistentVolumeClaims for your Dockerized application. If you need different kind of resources, use the 'kompose convert' and 'kubectl create -f' commands instead. 
 
INFO Deploying application in "default" namespace 
INFO Successfully created Service: redis          
FATA Error while deploying application: Deployment.apps "redis" is invalid: [spec.template.spec.containers[0].env[0].valueFrom.configMapKeyRef.name: Invalid value: "-/bar-env": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'), spec.template.spec.containers[0].env[1].valueFrom.configMapKeyRef.name: Invalid value: "-/bar-env": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')] 

ContainerName string
Image string `compose:"image"`
Environment []EnvVar `compose:"environment"`
EnvFile []string `compose:""`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can add compose:"env_file" as it is valid key

@surajnarwade
Copy link
Contributor

version 2 docker -compose file,

version: '3'
  
services:
  redis:
    image: 'bitnami/redis:latest'
    environment:
      - ALLOW_EMPTY_PASSWORD=no
    # Env file will override environment / warn!
    env_file:
      - ./bar.env
    labels:
      kompose.service.type: nodeport
    ports:
      - '6379:6379'

doesn't create configmaps,

$ kompose convert 
INFO Kubernetes file "redis-service.yaml" created 
INFO Kubernetes file "redis-deployment.yaml" created 

@cdrage
Copy link
Member Author

cdrage commented Aug 30, 2017

Your example shows Version 3 @surajnarwade

Yeah, at the moment it doesn't support Version 2. I will have to add that. Baby steps! 👍

Can you do a review on the basis Version 3?

@surajnarwade
Copy link
Contributor

@cdrage , my bad about version

👍 for baby step

@cdrage
Copy link
Member Author

cdrage commented Sep 5, 2017

@containscafeine @kadel @surajnarwade @surajssd ready for review 👍

edit: next sprint I will add v2 support, i have updated the conversion.md matrix accordingly.

@cdrage
Copy link
Member Author

cdrage commented Sep 6, 2017

Reason why this is not in v2 is because of this: docker/libcompose#488

@concaf
Copy link
Contributor

concaf commented Sep 7, 2017

@cdrage still getting the error in #799 (review) :(

@cdrage cdrage force-pushed the add-configmap branch 2 times, most recently from 7fc8fc8 to b0f2b00 Compare September 7, 2017 12:50
@cdrage
Copy link
Member Author

cdrage commented Sep 7, 2017

@containscafeine should work now, i've created a new function in order to format the new env name's correctly, feel free to do another review!

github.com/kubernetes/kompose  add-configmap ✗                                                                                                                                                                                                                          19m ⚑ ◒  
▶ ./kompose up
INFO We are going to create Kubernetes Deployments, Services and PersistentVolumeClaims for your Dockerized application. If you need different kind of resources, use the 'kompose convert' and 'kubectl create -f' commands instead. 
 
INFO Deploying application in "default" namespace 
INFO Successfully created Service: redis          
INFO Successfully created Deployment: redis       
INFO Successfully created Config Map: bar-env     

Your application has been deployed to Kubernetes. You can run 'kubectl get deployment,svc,pods,pvc' for details.

github.com/kubernetes/kompose  add-configmap ✗                                                                                                                                                                                                                          19m ⚑ ◒  
▶ kubectl get cm
NAME      DATA      AGE
bar-env   2         5s

@cdrage
Copy link
Member Author

cdrage commented Sep 26, 2017

ignoring vendoring updates, this PR needs review

edit: vendoring updates blocked on #835 due to running into errors with logrus + gojsonschema

@kadel @containscafeine @surajnarwade @surajssd please review

@cdrage cdrage force-pushed the add-configmap branch 4 times, most recently from 9b8e8d1 to 27de17c Compare September 29, 2017 14:54
@cdrage cdrage force-pushed the add-configmap branch 3 times, most recently from 1bc04b5 to 51bece6 Compare September 29, 2017 16:02
APIVersion: "v1",
},
ObjectMeta: api.ObjectMeta{
Name: envName,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be named like other objects kompose creates? appName - maifest type.yaml

Here also we can use same format, just add appName before envName ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so appName-configmap-env?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdrage this sounds good!

@surajnarwade
Copy link
Contributor

works for me 👍

When using env_file with Docker Compose, a ConfigMap will be generated

For example:

```sh
▶ ./kompose convert -f
script/test/fixtures/configmaps/docker-compose.yml
INFO Kubernetes file "redis-service.yaml" created
INFO Kubernetes file "redis-deployment.yaml" created
INFO Kubernetes file "foo-env-configmap.yaml" created
INFO Kubernetes file "bar-env-configmap.yaml" created
```

File:

```yaml
version: '3'

services:
  redis:
    image: 'bitnami/redis:latest'
    environment:
      - ALLOW_EMPTY_PASSWORD=no
    # Env file will override environment / warn!
    env_file:
      - "foo.env"
      - bar.env
    labels:
      kompose.service.type: nodeport
    ports:
      - '6379:6379'
```

To:

```yaml
apiVersion: v1
data:
  ALLOW_EMPTY_PASSWORD: "yes"
kind: ConfigMap
metadata:
  creationTimestamp: null
  name: foo-env
```

```yaml
...
      - env:
        - name: ALLOW_EMPTY_PASSWORD
          valueFrom:
            configMapKeyRef:
              key: ALLOW_EMPTY_PASSWORD
              name: foo-env
```
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. rebase needed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants