drone-gae icon indicating copy to clipboard operation
drone-gae copied to clipboard

MaxVersions tries to delete versions from other services when used with default service

Open RayBB opened this issue 3 years ago • 2 comments

Steps to reproduce:

  1. Deploy the default service (we'll call it frontend) to the GAE project
  2. Deploy the api service to the GAE project
  3. Deploy a 2nd version of the api service to the GAE project
  4. Deploy a 2nd version of the frontend service to the GAE project with maxVersions: 1

When you complete step 4 you will see it delete 2 services instead of 1. One from frontend and one from backend.

If you do the same but switch the deploy order of frontend (default) and backend (such that it's BFFB) then you will see the last step will only delete 1 service.

here is a sample drone where it will delete from other services yaml:

---
kind: pipeline
name: default

platform:
  os: linux
  arch: amd64

image_pull_secrets:
  - .dockerconfigjson

steps:

  - name: deploy_dev_frontend_default1
    pull: if-not-exists
    image: nytimes/drone-gae
    settings:
      action: deploy
      app_file: frontend.yaml #THIS IS THE frontend (default) SERVICE
      dir: frontend_src
      max_versions: 1
      project: nyt-project-dev
    environment:
      GAE_CREDENTIALS:
        from_secret: google_credentials_dev
    when:
      event: [push]

  - name: deploy_dev_backend1
    pull: if-not-exists
    image: nytimes/drone-gae
    settings:
      action: deploy
      app_file: backend.yaml #THIS IS THE API SERVICE
      dir: frontend_src
      max_versions: 1
      project: nyt-project-dev
    environment:
      GAE_CREDENTIALS:
        from_secret: google_credentials_dev
    when:
      event: [push]


  - name: deploy_dev_backend2
    pull: if-not-exists
    image: nytimes/drone-gae
    settings:
      action: deploy
      app_file: backend.yaml #THIS IS THE API SERVICE
      dir: frontend_src
      max_versions: 2
      project: nyt-project-dev
    environment:
      GAE_CREDENTIALS:
        from_secret: google_credentials_dev
    when:
      event: [push]

  - name: deploy_dev_frontend_default2
    pull: if-not-exists
    image: nytimes/drone-gae
    settings:
      action: deploy
      app_file: frontend.yaml #THIS IS THE frontend (default) SERVICE
      dir: frontend_src
      max_versions: 1
      project: nyt-project-dev
    environment:
      GAE_CREDENTIALS:
        from_secret: google_credentials_dev
    when:
      event: [push]

Here's a sample of the default an non-default swapped with no issue:

---
kind: pipeline
name: default

platform:
  os: linux
  arch: amd64

image_pull_secrets:
  - .dockerconfigjson

steps:

  - name: deploy_dev_backend1
    pull: if-not-exists
    image: nytimes/drone-gae
    settings:
      action: deploy
      app_file: backend.yaml #THIS IS THE API SERVICE
      dir: frontend_src
      max_versions: 1
      project: nyt-project-dev
    environment:
      GAE_CREDENTIALS:
        from_secret: google_credentials_dev
    when:
      event: [push]

  - name: deploy_dev_frontend_default1
    pull: if-not-exists
    image: nytimes/drone-gae
    settings:
      action: deploy
      app_file: frontend.yaml #THIS IS THE frontend (default) SERVICE
      dir: frontend_src
      max_versions: 1
      project: nyt-project-dev
    environment:
      GAE_CREDENTIALS:
        from_secret: google_credentials_dev
    when:
      event: [push]

  - name: deploy_dev_frontend_default2
    pull: if-not-exists
    image: nytimes/drone-gae
    settings:
      action: deploy
      app_file: frontend.yaml #THIS IS THE frontend (default) SERVICE
      dir: frontend_src
      max_versions: 2
      project: nyt-project-dev
    environment:
      GAE_CREDENTIALS:
        from_secret: google_credentials_dev
    when:
      event: [push]

  - name: deploy_dev_backend2
    pull: if-not-exists
    image: nytimes/drone-gae
    settings:
      action: deploy
      app_file: backend.yaml #THIS IS THE API SERVICE
      dir: frontend_src
      max_versions: 1
      project: nyt-project-dev
    environment:
      GAE_CREDENTIALS:
        from_secret: google_credentials_dev
    when:
      event: [push]

Edit: I've since confirmed that setting "service: default" in the drone.yaml or in the app.yaml will stop this plugin from deleting versions of all other services. However, that should probably happen without user having to specify.

RayBB avatar Jul 13 '20 22:07 RayBB

I've since confirmed that setting "service: default" in the drone.yaml or in the app.yaml will stop this plugin from deleting versions of all other services.

That's an interesting edge case. service: isn't required for the project's default service in app.yaml, but I wouldn't expect operations on the default service to affect other services.

I suppose we could update the plugin to log a warning if service isn't specified in either the app.yaml or the drone.yaml so that a user knows what's happening.

brianfoshee avatar Jul 20 '20 13:07 brianfoshee

Do you think it would make sense to fix it so if no service is specified it:

  1. Automatically assumes the service is default (like gcloud does)
  2. Throws an error
  3. Throws a warning

Answer (from Brian):

I think 1 and 2 would be a breaking change in the plugin and require a major version bump, in case anyone is relying on that behavior. I'm not opposed to 1 though, setting service to default if none is specified in app or drone yaml files. If your team wants to PR that or open an issue with the proposed solution we can at least have a conversation about it to see if any other users are opposed to the behavior.

RayBB avatar Jul 20 '20 14:07 RayBB