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 Plugins doesn't execute V2 commands when calling CliConnection.CliCommand #1445

Closed
simonjohansson opened this issue Aug 23, 2018 · 12 comments
Labels

Comments

@simonjohansson
Copy link
Contributor

What are we trying to achieve

We are maintaining a cf plugin that does zero downtime deployment, today when we tried the new buildpacks feature that 6.38 gives us we got greeted by

$ cf halfpipe-push -manifestPath manifest.yml -appPath=. -testDomain=asd.com -space=halfpipe
# CF plugin built from git revision '7a502a6c20808565f3c692e287fb62a87a17caf8'
# Planned execution
#	* cf push multiple-buildpacks-test-CANDIDATE -f manifest.yml -p . --no-route --no-start
#	* cf map-route multiple-buildpacks-test-CANDIDATE asd.com -n multiple-buildpacks-test-halfpipe-CANDIDATE
#	* cf start multiple-buildpacks-test-CANDIDATE

$ cf push multiple-buildpacks-test-CANDIDATE -f manifest.yml -p . --no-route --no-start
Error reading manifest file:
The following manifest fields cannot be used with legacy push: buildpacks

Which is strange, since copy pasting cf push multiple-buildpacks-test-CANDIDATE -f manifest.yml -p . --no-route --no-start and executing it directly works like expected.

What occurred

Since halfpipe-push is not a core command,handleFlagErrorAndCommandHelp redirects us to cmd.Main which in turn start a RPC server and runs my plugin. When my plugin calls out to CliConnection.CliCommand("push" ....) the CliRpcCmd.CallCoreCommand will iterate over the available commands in commandregistry.Commands (who's commands have been populated via init() methods at startup) and then finally execute "legacy" push from cf/commands/application/push.go

What you expected to occur

The RPC server calls out to command/v2/push_command.go instead of cf/commands/application/push.go so that I can push an app with a manifest containing the buildpacks key

CLI Version

cf version 6.38.0+7ddf0aadd.2018-08-07

Plugin built with

$ cat Gopkg.lock
.....
[[projects]]
  branch = "master"
  name = "github.com/lunixbochs/vtclean"
  name = "code.cloudfoundry.org/cli"
  packages = [
    "actor/actionerror",
    "api/cloudcontroller/ccerror",
    "api/cloudcontroller/ccv2/constant",
    "api/plugin/pluginerror",
    "api/uaa",
    "api/uaa/constant",
    "api/uaa/internal",
    "cf/errors",
    "cf/i18n",
    "command/translatableerror",
    "i18n/resources",
    "plugin",
    "plugin/models",
    "types",
    "util",
    "util/clissh/ssherror",
    "util/configv3",
    "util/download",
    "util/manifest",
    "util/ui",
    "version"
  ]
  revision = "9048c6b553bb90bdd92d4156d7e6a54b82a45e72"
  version = "v6.38.0"
@cf-gitbot
Copy link

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

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

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

@simonjohansson
Copy link
Contributor Author

Here is a simpler plugin that shows the issue.

package main

import (
	"code.cloudfoundry.org/cli/plugin"
	"fmt"
)

type Yoyo struct{}

func (Yoyo) Run(cliConnection plugin.CliConnection, args []string) {
	_, err := cliConnection.CliCommand("push")
	fmt.Println(err)
}

func (Yoyo) GetMetadata() plugin.PluginMetadata {
	return plugin.PluginMetadata{
		Name: "yoyo",
		Commands: []plugin.Command{
			{
				Name: "yoyo",
			},
		},
	}
}

func main() {
	cfPlugin.Start(new(Yoyo))
}

If run in a folder with two files, manifest.yml and server.js

---
applications:
- name: buildpacks-test
  buildpacks:
    - https://github.com/cloudfoundry/nodejs-buildpack
  command: node server.js
const http = require('http');
const server = http.createServer((req, res) => {
    res.end('ok');
});
server.listen(8080);

We get the same error as reported above

$ cf yoyo
Error reading manifest file:
The following manifest fields cannot be used with legacy push: buildpacks

@abbyachau
Copy link
Contributor

Hi @simonjohansson what version of cc api are you on?

@simonjohansson
Copy link
Contributor Author

@abbyachau

We have a couple of installations, our latest on is at 2.114.0 and our oldest one is at 2.96.0.

@abbyachau
Copy link
Contributor

abbyachau commented Aug 28, 2018

Hey @simonjohansson sorry v3 features (multiple buildpacks) do not currently work with our plugin architecture. Please see this verbose explanation as to why that is currently the case. I've updated the plugin documentation with this known issue.

Please also note we are collecting information here if you are interested in providing feedback. Closing this issue but feel free to reopen/create a new issue if you have further questions, thanks.

@simonjohansson
Copy link
Contributor Author

simonjohansson commented Aug 28, 2018

@abbyachau thanks for the update.

Btw, verbose explanation link is points to the wrong issue. :) Here is the correct link

Btw2, if you want to collect any use cases from us on how/why we use the plugin architecture just reach out. Would also be happy to help.

@CAFxX
Copy link

CAFxX commented Sep 6, 2018

@abbyachau we just ran into the same problem (we are trying to do cf push --droplet ... from a plugin). Lack of support is made even worse by the fact that there's no viable cf curl fallback to push a droplet, AFAIK.

Unfortunately, I can't add comment/feedback on https://www.pivotaltracker.com/n/projects/892938/stories/157201049

@abbyachau
Copy link
Contributor

Hi @simonjohansson thanks, will do.

@CAFxX thanks, I've added your comment to our tracker story.

@abbyachau
Copy link
Contributor

abbyachau commented Dec 6, 2018

Hi @simonjohansson looking forward to speaking directly. I have a few preliminary questions: could you share your entire manifest with us? Are you actually using multiple buildpacks (the example above is only showing one)? If you change your manifest from:

buildpacks:
    - https://github.com/cloudfoundry/nodejs-buildpack

to

buildpack:
    - https://github.com/cloudfoundry/nodejs-buildpack

does that work at all? Thanks in advance.

@abbyachau
Copy link
Contributor

Hi all, the CF CLI team are exploring how we would be able to help plugin authors with the issues described in this thread - specifically the issue that the plugin architecture was designed only to call the legacy codebase, and as such new features and refactored code are inaccessible to plugin authors. To that end, the CLI eng team wrote an Exploration detailing a couple of solutions on how to move forward. I’m reaching out to folks in the Community who have expressed issues with plugins to understand:

  1. if the first two solutions detailed in the doc would solve the issue you’ve encountered (unable to use --var, --droplet, buildpacks, etc` - and if so, which solution would best solve your use case?

  2. OR do we need to provide additional technical details in the doc so that you are able to comment with a preferred solution? The goal is to gain confidence that either solution will meet the needs of the Community. If there is not enough information in the document, we'll add additional details.

If you can comment in the document, that would be helpful as well. Thanks very much for your time, and please do not hesitate to reach out if you have any questions.

@abbyachau
Copy link
Contributor

Hi @simonjohansson, was there a workaround for multiple buildpacks that you found that you could share here?

@simonjohansson
Copy link
Contributor Author

simonjohansson commented May 15, 2019

Hi @abbyachau yeah, we worked around it.

We use a command pattern in our plugin and instead of using CliConnection.CliCommand("command", ....) in our executor we shell out to "cf command".. :)

I.e

https://github.com/springernature/halfpipe-cf-plugin/blob/master/plan/executor.go#L21

used to be

https://github.com/springernature/halfpipe-cf-plugin/blob/e675b90fa47a048a7a713e5a0706fa4ca1bedfd4/plan/plan.go#L35

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

No branches or pull requests

4 participants