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

CLI throws faulty error when outdir is passed to AppProps #210

Closed
pmkuny opened this issue Feb 24, 2022 · 7 comments · Fixed by cdk8s-team/cdk8s-core#783
Closed

CLI throws faulty error when outdir is passed to AppProps #210

pmkuny opened this issue Feb 24, 2022 · 7 comments · Fixed by cdk8s-team/cdk8s-core#783
Assignees
Labels
bug Something isn't working effort/medium 1 week tops priority/p1 Should be on near term plans

Comments

@pmkuny
Copy link

pmkuny commented Feb 24, 2022

Description of the bug:

Configuration of the output directory outdir for manifests file in the App construct results in an error message with the default output directory (dist)

I am using the following:

  • Python 3.7.10
  • cdk8s-cli 1.0.107
  • cdk8s==1.5.8
  • cdk8s-plus-22==1.0.0b109

Reproduction Steps:

from constructs import Construct
from cdk8s import App, Chart
app = App(
    outdir="manifests"
)

Error Log:

After running cdk8s synth:

ERROR: synthesis failed, app expected to create "dist"

Environment:

  • Framework Version: 1.5.8
  • OS: AL2
attrs==21.4.0
cattrs==1.10.0
cdk8s==1.5.8
cdk8s-plus-22==1.0.0b109
constructs==3.3.209
jsii==1.52.1
publication==0.0.3
python-dateutil==2.8.2
six==1.16.0
typing_extensions==4.0.1

Other:


This is 🐛 Bug Report

@pmkuny pmkuny added bug Something isn't working needs-triage Priority and effort undetermined yet labels Feb 24, 2022
@Chriscbr Chriscbr removed the needs-triage Priority and effort undetermined yet label Feb 25, 2022
@Chriscbr
Copy link
Contributor

Hmm - I believe this might just be a problem with a faulty error message. When I reproduce this, I get the same error message, but the yaml file does appear in the directory I specified. (Let me know if you observe different)

I think this is probably a bug in the command line tool...

@pmkuny
Copy link
Author

pmkuny commented Feb 25, 2022

@Chriscbr - You are correct, I just checked and I'm getting the same result. Error message but seeing the rendered manifests under the correct directory specified in the App() property. Would you like me to leave this open and edit it for clarification, or close?

@Chriscbr
Copy link
Contributor

@pmkuny Editing it is fine -- it's still a bug for sure! 😉

@pmkuny
Copy link
Author

pmkuny commented Feb 25, 2022

@Chriscbr - Should I create an issue in cdk8s-cli and link it here, or should I just edit this to track that?

@Chriscbr Chriscbr transferred this issue from cdk8s-team/cdk8s Feb 25, 2022
@Chriscbr Chriscbr changed the title AppProps being ignored when passing into App CLI throws faulty error when outdir is passed to AppProps Feb 25, 2022
@Chriscbr
Copy link
Contributor

@pmkuny I've gone ahead and transferred the issue straight over. Thanks!

@martimors
Copy link

Faced this peculiar error message

ERROR: synthesis failed, app expected to create "dist"

although everything seems fine. The problem is that the exit code is 1, so this causes CI pipelines to fail. Using the default dist/ directory for now. Just thought I'd chime in that this bug is still there in 2.0.105.

@iliapolo iliapolo added priority/p1 Should be on near term plans effort/small 1 day tops labels Sep 18, 2022
@iliapolo iliapolo added effort/medium 1 week tops and removed effort/small 1 day tops labels Sep 28, 2022
@iliapolo
Copy link
Member

Yeah this is a silly one. Basically when the outdir is configured on the App instance, the CLI currently doesn't know about it, so it still thinks dist was used, and validates against that.

We are looking into it, but for anyone interested in using a custom output directory, you can configure it using the output property in cdk8s.yaml, instead of the App instance. That should work fine.

mergify bot pushed a commit to cdk8s-team/cdk8s-core that referenced this issue Oct 4, 2022
… file (#783)

Currently setting the `outdir` property for CDK8s app throws an error,
```
ERROR: synthesis failed, app expected to create "dist"
```

This is due to the CLI not able to recognize this change in the `outdir` in the application property. 
The value specified in the `outdir` needs to be similar to what is mentioned either in CLI's `--output` argument or the `output` property in the cdk8s.yaml file. 

Signed-off-by: Vinayak Kukreja <[email protected]>

Resolves cdk8s-team/cdk8s-cli#210
cdk8s-automation pushed a commit to cdk8s-team/cdk8s-core that referenced this issue Oct 4, 2022
… file (#783)

Currently setting the `outdir` property for CDK8s app throws an error,
```
ERROR: synthesis failed, app expected to create "dist"
```

This is due to the CLI not able to recognize this change in the `outdir` in the application property.
The value specified in the `outdir` needs to be similar to what is mentioned either in CLI's `--output` argument or the `output` property in the cdk8s.yaml file.

Signed-off-by: Vinayak Kukreja <[email protected]>

Resolves cdk8s-team/cdk8s-cli#210

(cherry picked from commit 05bf5f4)
Signed-off-by: Vinayak Kukreja <[email protected]>
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.

5 participants