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

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

Open
RayBB opened this issue Jul 13, 2020 · 2 comments

Comments

@RayBB
Copy link

RayBB commented Jul 13, 2020

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.

@brianfoshee
Copy link
Contributor

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.

@RayBB
Copy link
Author

RayBB commented Jul 20, 2020

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.

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

2 participants