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

fix(util): correct logging for synth #1634

Open
wants to merge 9 commits into
base: 2.x
Choose a base branch
from

Conversation

iceglober
Copy link

@iceglober iceglober commented Nov 10, 2023

FYI - This is my first OSS contribution. I'm open to any feedback or input!

Summary of Changes

The console logs generated during cdk8s synth gave the impression that synthesis had failed when it actually succeeded, leading to unnecessary confusion. This change updates that log to a warning if the outdir doesn't exist, and also adds a "success" log if generated manifests are found. This isn't actually a problem because synth in cdk8s-core always attempts to create the necessary directory.

Current State

Without providing a CLI option for outdir and without specifying an outdir property in my config file, I defined my App like so:

const app = new App({
    outdir: 'out',
    yamlOutputType: YamlOutputType.FOLDER_PER_CHART_FILE_PER_RESOURCE,
});
new MongoDbChart(app, 'mongoDb2');
app.synth();

where outdir is defined in my IaC directly. I wanted to make a clear differentiation between TS output ("dist") and cdk8s output (I chose "out").

With this current state, here's what the console output looks like:

iceglobe@iceglobe-MS-7C79 ~/projects/kayn/k8s/cdk8s
 % cdk8s synth
Synthesizing application
ERROR: synthesis failed, app expected to create "dist"

This "ERROR" message gives the false impression that manifest synthesis failed, when in reality, because I had specified a different outdir in my TS file, everything worked just fine.

Proposed Change

My changes make it so that the console output reflects the following:

iceglobe@iceglobe-MS-7C79 ~/projects/kayn/k8s/cdk8s
 % cdk8s synth
WARNING: specified outdir does not exist, synth command could fail
Synthesizing application
  - dist/mongodb2-c84b3744/Deployment.mongodb.k8s.yaml
  - dist/mongodb2-c84b3744/Secret.mongodb-secret.k8s.yaml
  - dist/mongodb2-c84b3744/Service.mongodb-service.k8s.yaml
Manifests synthesized!

This accomplishes the following:

  • Gives warning that synth might fail depending on the shell command that is run
  • Allows intended logging of generated manifests to be displayed (this was previously inhibited because the pathExists check resulted in an exit)
  • Gives an affirmative message that manifests were indeed generated as expected ("Manifests synthesized!")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant