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

Duplicated synthed resources when nesting Charts #31

Closed
benagricola opened this issue Nov 9, 2020 · 0 comments · Fixed by #40
Closed

Duplicated synthed resources when nesting Charts #31

benagricola opened this issue Nov 9, 2020 · 0 comments · Fixed by #40
Labels
bug Something isn't working effort/medium 1 week tops priority/p1 Should be on near term plans

Comments

@benagricola
Copy link

Description of the bug:

When nesting charts within a parent chart, applying the parent chart to an app and then synthesizing the app, I'm seeing duplicated synthed resources - one file for each separate chart nested in the parent chart, and then one file for the parent chart which also includes the resources from the nested charts.

This is problematic when applying to Kubernetes using kubectl apply -f dist/ as the files contain duplicate identically named resources. Not sure if this is expected or not but submitting as a bug report just in case 🙂

Reproduction Steps:

app/test.ts:

import { App, Chart } from 'cdk8s'
import { Construct } from 'constructs'
import { Namespace } from '../imports/k8s';


export class ChildChart1 extends Chart {
    constructor(scope: Construct, name: string) {
      super(scope, name)
      const ns1 = new Namespace(this, 'namespace1')
    }
}

export class ChildChart2 extends Chart {
    constructor(scope: Construct, name: string) {
      super(scope, name)
      const ns2 = new Namespace(this, 'namespace2')
    }
}

export class ParentChart extends Chart {
    constructor(scope: Construct, name: string) {
        super(scope, name)
        const childChart1 = new ChildChart1(this, 'child1')
        const childChart2 = new ChildChart2(this, 'child2')
        const ns3 = new Namespace(this, 'namespace3')
    }
}
const app = new App()
const parent = new ParentChart(app, 'parent')
app.synth()

shell:

➜  cdk8s git:(feat/nested-children) ✗ cdk8s synth -a "npx ts-node -r tsconfig-paths/register app/test.ts"
dist/parent-child1-cad88a69.k8s.yaml
dist/parent-child2-1e516eb8.k8s.yaml
dist/parent.k8s.yaml

➜  cdk8s git:(feat/nested-children) ✗ cat dist/parent-child1-cad88a69.k8s.yaml
apiVersion: "v1"
kind: "Namespace"
metadata:
  name: "parent-child1-namespace1-e27612e5"

➜  cdk8s git:(feat/nested-children) ✗ cat dist/parent-child2-1e516eb8.k8s.yaml
apiVersion: "v1"
kind: "Namespace"
metadata:
  name: "parent-child2-namespace2-32aecabe"

➜  cdk8s git:(feat/nested-children) ✗ cat dist/parent.k8s.yaml
apiVersion: "v1"
kind: "Namespace"
metadata:
  name: "parent-child1-namespace1-e27612e5"
---
apiVersion: "v1"
kind: "Namespace"
metadata:
  name: "parent-child2-namespace2-32aecabe"
---
apiVersion: "v1"
kind: "Namespace"
metadata:
  name: "parent-namespace3-7362c69c"

Environment:

cdk8s-test@0.0.1 
├── @types/jest@26.0.15
├── @types/node@14.14.6
├── cdk8s@0.33.0
├── cdk8s-cli@0.33.0
├── cdk8s-plus@0.33.0
├── constructs@3.2.11
├── dotenv@8.2.0
├── jest@26.6.3
├── npm-run-all@4.1.5
├── source-map@0.7.3
├── source-map-support@0.5.19
├── ts-jest@26.4.3
├── ts-node@9.0.0
├── tsconfig-paths@3.9.0
└── typescript@4.0.5

Other:

I'm not bothered what the correct output is here. It seems to me that output should either be in individual files (one per nested child, with the parent resources in their own file) or all resources should be put into the one parent file. I don't think it matters, as long as the directory itself doesn't contain duplicate resources.


This is 🐛 Bug Report

@iliapolo iliapolo changed the title Duplicated synthed resources when nesting Charts [cdk8s] Duplicated synthed resources when nesting Charts Nov 22, 2020
@iliapolo iliapolo transferred this issue from cdk8s-team/cdk8s Jul 6, 2021
@iliapolo iliapolo added bug Something isn't working effort/medium 1 week tops priority/p1 Should be on near term plans labels Jul 6, 2021
@iliapolo iliapolo removed their assignment Jul 6, 2021
@iliapolo iliapolo changed the title [cdk8s] Duplicated synthed resources when nesting Charts Duplicated synthed resources when nesting Charts Jul 6, 2021
@mergify mergify bot closed this as completed in #40 Jul 29, 2021
mergify bot pushed a commit that referenced this issue Jul 29, 2021
Fixes #31

I think the expected behavior from users is that in most circumstances, you should be able to directly apply resources generated by `cdk8s synth` to your k8s cluster (e.g. via `kubectl apply -f dist/`). However, it's possible for nested charts to cause resources to be listed twice. This PR resolves this by de-duplicating resources so that each one is only rendered once during a given "synth" call.

This PR makes the assumption that nesting charts within each other is implicitly a feature (I don't believe there are any other ways to arrive at charts containing the same resource twice?). It's possible we could instead add some kind of validation to prevent charts from being nested to resolve this issue instead, but this would become problematic if users are defining constructs that include charts.

Questions:
- Should there be some way to synthesize a specific chart within an app from the CLI?
- Should there be an option for synthesis (e.g. a CLI flag) to override this and allow duplicate objects?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working effort/medium 1 week tops priority/p1 Should be on near term plans
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants