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

cf cli 6.37.0: --vars-file and --var 'invalid flag' #1399

Closed
unnmdnwb3 opened this issue Jun 14, 2018 · 12 comments
Closed

cf cli 6.37.0: --vars-file and --var 'invalid flag' #1399

unnmdnwb3 opened this issue Jun 14, 2018 · 12 comments

Comments

@unnmdnwb3
Copy link

hey everyone, I got some issues with the --vars-file and --var... thanks in advance!

Command

cf push frontend -p frontend/dist -f ./deployment-manifests/demo-launchpad/DEV/DEV-frontend-manifest.yml --vars-file ./deployment-manifests/demo-launchpad/DEV/DEV-config.yml

and cf push frontend -p frontend/dist -f ./deployment-manifests/demo-launchpad/DEV/DEV-frontend-manifest.yml --var customer-id=test

What occurred

Invalid flag: --vars-file & Invalid flag: --var

What you expected to occur

Variable Substitution as described in: https://docs.cloudfoundry.org/devguide/deploy-apps/manifest.html

CLI Version

cf.exe version 6.37.0+a40009753.2018-05-25

CC API Endpoint Version

2.107.0

Platform & Shell Details

Microsoft Windows Enterprise 64-bit with MINGW64 and CMD

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/158361861

The labels on this github issue will be updated when the story is started.

@gsmachado
Copy link

@unnmdnwb3 I'm having EXACTLY the same behavior as you described. That's somehow sad since I was eager for having such feature.

My operating system is MacOS 10.12.6, and using 6.37.0+a40009753.2018-05-25.

@abbyachau
Copy link
Contributor

Hi @unnmdnwb3 thanks for creating this issue. Could you try the following instead --var=customer-id=test - let us know if you still get the same flag errors. Thanks.

@abbyachau
Copy link
Contributor

abbyachau commented Jun 14, 2018

Hey @unnmdnwb3 @gsmachado we figured out that you might be using the variable substitution features in a plugin, which we do not support. Let us know if that's the case, thanks.

@abbyachau
Copy link
Contributor

Note: It appears that if variable substitution is used with deprecated properties (no-route or health-check-type: none, for example), then users will see the errors described above. We are updating the error message to indicate that variable substitution cannot be used alongside deprecated properties.

@unnmdnwb3
Copy link
Author

hey @abbyachau, thanks a lot for your answer and sorry for replying so late, I was away for a few days.

you were absolutely right - it's working now!

the problem was this: Deprecation warning: Specifying app manifest attributes at the top level is deprecated. Found: disk_quota, timeout, instances, memory.

after specifying these attributes below the name, I didn't get this warning anymore.
and since this was resolved, I could use the variable substitution without any problems.

thanks alot again! :)

@abbyachau
Copy link
Contributor

Awesome @unnmdnwb3 great to hear, thanks for the update. We updated command line output to give a warning that using variable substitution and deprecated values is not allowed. Hope that helps reduce confusion in the future.

@gsmachado
Copy link

@abbyachau thanks for taking a look at it and with a fast response. Much appreciated. :-)
It's also working for me now after removing the deprecated attributes.

@bpandola
Copy link

@abbyachau You say variable substitution is not supported in plugins... Will it be supported in the future or is there some reason why this isn't/won't be allowed?

@XenoPhex
Copy link
Contributor

Hey there @bpandola,

The [really] short answer is that all plugins use a legacy code path that does not have access to any new CLI feature as of CF 6.25+. This is due to an internal re-architecture that's designed to clean up the CLI source without affecting any existing plugin. If this satisfies you're curiosity - than feel free to stop reading here, if not then continue on.

The long[er] answer requires understanding how the overall CLI architecture. Again, to keep things brief I'll summarize as much as I can. Architecture wise:

  • The CLI is divided into two sections - Legacy and Refactored code.
  • The Legacy code is everything underneath the cf directory and this code is what the plugin architecture hooks into. This code currently uses a custom Command Line Options Parser (copied from codegangsta/cli) and does a lot of wacky stuff. To add to it, there is a lot of inconsistent and hard to follow code there that made even experienced GoLang developers shy away from the project.
  • The Refactored code is 90% of the code outside of the cf directory. This code is not hooked into the plugin architecture because it uses go-flags as a Command Line Options Parser. The reason the Refactored code exists is due to the a lack of test coverage, idiomatic GoLang patters, and general difficulty of working with the Legacy code. A decision was made in mid-2016 to do an "in-place rewrite" of every command in order address these problems as well as make it easier for the developers to add new features going forward.
  • Unfortunately the plugin architecture is so tightly coupled to the Legacy code framework that it would take a pair (or more) several months to: wrap their heads around how the plugin framework works; decouple it from the Legacy code; and then provide some sort of smart switching mechanism between the Legacy and Refactored code.
  • In order to reduce the potential problems for existing plugins as the CLI team cleans up it's code (ie, expand the Refactored code) - the decision was made not to attach the plugin code to the Refactored code. A side effect of this decision means that any new feature in the Refactored code is inaccessible to the plugin architecture.

@abbyachau
Copy link
Contributor

Closing this issue as we will release cf cli v6.38.0 soon which will include a warning if you attempt to use variable substitution with deprecated values. If any feedback/additional comments, feel free to reopen or create a new issue.

@bpandola
Copy link

bpandola commented Aug 2, 2018

@XenoPhex While I'm disappointed that plugins will not have access to newer CLI features, I do very much appreciate the detailed response (and will move forward with a workaround). Thanks.

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

No branches or pull requests

6 participants