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

Support printing to stdout #5

Merged
merged 1 commit into from
Jul 3, 2016
Merged

Conversation

janetkuo
Copy link
Member

@janetkuo janetkuo commented Jul 1, 2016

Fixes #2

Add a -out / -stdout flag for printing to stdout. Doesn't support printing helm chart since it doesn't make sense (or does it?). I think the naming of the flag could be better but I couldn't think of a better one for now.

Some unsolved issues found while developing:

  1. Services should be created first if possible
  2. It seems output to helm chart is only supported when there's xxx-rc.json (not yaml, not other controllers)
  3. Right now we don't stop users from creating more than one type of controllers (rc, rs, deployment, ...), but this won't work while printing to stdout and pipe to kubectl, so I changed the logic to just use the first flag specified. Does that sound good? We'll need some verification for incorrect flag usage also.

@Runseb @ngtuna

@ngtuna ngtuna merged commit fd82f2c into kubernetes:master Jul 3, 2016
@ngtuna
Copy link
Contributor

ngtuna commented Jul 3, 2016

Thanks @janetkuo . LGTM.

1 - Yes agree, services should be created first, specially when writing them to stdout help the pipe to kubectl working better.
3 - Yes, should have a verification on input flags and also printing a warning. The logic IMO would be only accepting the first flag in case of printing to stdout, otherwise keep the current support.

@ngtuna
Copy link
Contributor

ngtuna commented Jul 4, 2016

@janetkuo I kept output to stdout option flag --stdout (removed --out) and added output to single file with flag -o/--out at bea0e4b. They can't be declared together at the same time.

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.

2 participants