cloudfoundry-community / cf-resource

Concourse resource for interacting with Cloud Foundry
Apache License 2.0
35 stars 63 forks source link

Unable to use new manifests when using "current_app_name" param #62

Open ipsi opened 6 years ago

ipsi commented 6 years ago

If I have a put like

    - put: deploy-transaction
      resource: deploy-cf
      params:
        manifest: archive/transaction/manifest.yml
        current_app_name: transaction

And a manifest like

applications:
- name: transaction
  services:
  - redis
  buildpacks:
  - java_buildpack_offline
  memory: 1024M
  path: build/libs/transaction-1.0.0.jar

Then when deploying we get an error like:

Renaming app transaction to transaction-venerable in org MY_ORG / space temp as ci-user...
OK
Deleting app transaction in org MY_ORG / space temp as ci-user...
OK
App transaction does not exist.
Renaming app transaction-venerable to transaction in org MY_ORG / space temp as ci-user...
OK
error: Error reading manifest file:
The following manifest fields cannot be used with legacy push: buildpacks

error running command: exit status 1

Note that this works if we remove current_app_name, like so:

    - put: deploy-transaction
      resource: deploy-cf
      params:
        manifest: archive/transaction/manifest.yml

OR

If we change buildpacks to buildpack:

applications:
- name: transaction
  services:
  - redis
  buildpack: java_buildpack_offline
  memory: 1024M
  path: build/libs/transaction-1.0.0.jar
davidje13 commented 6 years ago

This also has the more problematic limitation that vars is incompatible with the legacy manifest format:

error: The following flags cannot be used with deprecated usage: var

This means we have to choose between the zero-downtime-deploy feature, and being able to use a single manifest per app.

davidje13 commented 6 years ago

This is caused by autopilot (https://github.com/contraband/autopilot/issues/59) which in turn points to cf cli's API (https://github.com/cloudfoundry/cli/issues/1445), finally pointing at a description of a massive refactor which has been causing problems like this for plugins (https://github.com/cloudfoundry/cli/issues/1399#issuecomment-409061226)

The short explanation is that this line in autopilot uses the CF CLI plugin code (as it should):

https://github.com/contraband/autopilot/blob/5e34f165d172fd1253070c6c7ada984a03cb447e/autopilot.go#L301

But the CF CLI plugin code is stuck using legacy code, and has no way to use non-legacy.

I think the simplest fix (but likely years away) would be to continue CF CLI's existing replicate-the-world approach and add a new non-legacy plugin API which plugins can choose to switch to. Then autopilot could switch to that, and this issue should be resolved. In that case, this problem would be entirely out of the hands of the cf-resource project.

Alternatively, cf-resource could replace autopilot with a series of raw cf commands doing exactly the same thing that the plugin is currently doing (since the CLI part of the CF CLI is using non-legacy codepaths). Translated into bash, the tasks appear to be:

APP_NAME="<app-name>";

app_state() {
  cf app "$1" | grep 'requested state:' | tr -s ' ' | cut -f3 -d' ' || echo 'missing';
}
VEN_NAME="${APP_NAME}-venerable";
APP_STATE="$(app_state "$APP_NAME")";
MADE_VEN='false';

if [[ "$APP_STATE" != 'missing' ]]; then
  if [[ "$APP_STATE" != 'started' ]]; then
    cf delete "$APP_NAME" -f;
  else
    VEN_STATE="$(app_state "$VEN_NAME")";
    if [[ "$VEN_STATE" != 'missing' ]]; then
      cf delete "$VEN_NAME" -f;
    fi;
    cf rename "$APP_NAME" "$VEN_NAME";
    MADE_VEN='true';
  fi;
fi;

if cf push "$APP_NAME" [extra args here...]; then
  if [[ "$MADE_VEN" == 'true' ]]; then
    cf delete "$VEN_NAME" -f;
  fi;
  echo 'SUCCESS!';
else
  if [[ "$MADE_VEN" == 'true' ]]; then
    cf logs "$APP_NAME" --recent; # (I added this; autopilot doesn't do it)
    cf delete "$APP_NAME" -f;
    cf rename "$VEN_NAME" "$APP_NAME";
  fi;
  echo 'FAILED :(';
fi;

It would take a bit of work to translate this from bash into go, and it will make PushApp much more complicated, but it would mean entirely bypassing CF CLI's plugin architecture, which means all these issues would go away.

abbyachau commented 5 years ago

Hi @ipsi thanks for pointing this issue out. As @davidje13 points out, our plugin architecture is tied to our legacy code, which does not interact nicely with refactored commands or new features. The CLI team did an exploration recently to try to get to an understanding as to how to help plugin authors but we do not have enough information to move forward, and also we want to understand how the new upcoming rolling deployments feature fits into the picture for some of the plugins which have reported issues.

Here is the tracker story for reference which includes a document detailing some of the engineers' findings. Please feel free to comment in the document.