Code Monkey home page Code Monkey logo

Comments (9)

toolmantim avatar toolmantim commented on July 28, 2024

Sorry, we didn’t realize the Anka plugin behaves like this. This plugin existed first, and is designed to try to closely match Docker’s semantics, and work across different platforms, and generally tries to avoid command string parsing/splitting. I can see how the combo is confusing though!

I’m not sure what a great answer is here. Perhaps we could add another option, or we could look at getting the Anka plugin modified?

Could you post a simplified side-by-side example YML here to compare and discuss options? It might be worth creating an issue on the Anka plugin side to cross reference here too.

from docker-buildkite-plugin.

NorseGaud avatar NorseGaud commented on July 28, 2024

Thanks Tim. I'm not sure I want to modify the Anka plugin; I like how it behaves and I've already put a lot of time into expanding the functionality.

This is more a concern with the way the docker plugin is doing things. What are the benefits of chaining listed commands instead of executing one by one? I'm curious if there is a docker reason why?

Off of the top of my head here are the benefits:

  1. By separating the command execution by each list item, logging is way better.
  2. Logically, seeing as you can chain commands within a single list item - "command1 && command2", chaining them inside of the plugin doesn't make sense.
  3. Mac/Anka and Linux steps defined in the pipeline.yml can match.

from docker-buildkite-plugin.

toolmantim avatar toolmantim commented on July 28, 2024

Thanks for the explanation… I can definitely see the use case.

Any idea how we'd handle this case? (which we handle, but there's no example in the readme or on https://buildkite.com/docs/pipelines/command-step#command-step-attributes I'm afraid, even though it's quite common ☹️)

steps:
  - command: |
      yarn install
      yarn run test
    plugins:
      - docker#v3.2.0:
          image: "node:10"

It's parsed as:

{
  "steps": [
    {
      "command": "yarn install\nyarn run test\n", 
      "plugins": [
        {
          "docker#v3.2.0": {
            "image": "node:10"
          }
        }
      ]
    }
  ]
}

Those newlines would then be run as completely separate containers in your case, which is very different semantics to all the other bits and pieces of Buildkite.

It'd be great if this plugin could support this use case though (running a bunch of commands in their own containers in sequence, ensuring each one returns correctly, and handles container log checking and all the other things it does now)… vs having to have separate pipeline steps. It's quite a big change though 🤔

from docker-buildkite-plugin.

NorseGaud avatar NorseGaud commented on July 28, 2024

Yep, that's actually an open issue in the anka-buildkite-plugin too. I have yet to sit down and really figure out how to parse it. I was told that command: | was a hacky not supported option for execution (is that true?).

I'm thinking about using IFS='\n' in combination with some check for && at the end of the line to split those into separate commands (or not, but remove \n). My thought process is that in a bash script, those would execute as separate commands.

I can take a crack at solving it for the anka plugin and then follow up with this issue to get some feedback and see about replacing the current execution logic in a major release for the docker plugin, sound good?

from docker-buildkite-plugin.

toolmantim avatar toolmantim commented on July 28, 2024

Thanks for the offer! That might be a good plan.

One thing we do support though, and we can’t really deprecate, is this:

steps:
  - command: |
      yarn install
      yarn run test

Many people are translating existing build steps/scripts to Buildkite from other systems, and even our web UI, that supported a textarea to mash commands.

We’ve had heaps of feedback from users that it should have “just worked” in the Docker plugins, which is why we kept the bash wrapping semantics and allowed another custom command plugin option that had different semantics (it does no bash wrapping, and is the equivalent to Docker’s CMD) which is different again to the semantics you’re shooting for here.

The other big concern I still have, config syntax aside, is making this plugin work for n container runs, and what it’d mean to actually implement. At a guess, it’d start to get much more complex.

I’m struggling to see a smooth path forward with the Anka style! But maybe there is 🤔 What were the reasons you wanted to support n container runs for the command block for the Anka plugin, instead of having people just define separate pipeline steps?

from docker-buildkite-plugin.

NorseGaud avatar NorseGaud commented on July 28, 2024

Understood, thanks for clarifying command: | for me.

I just now noticed that you're using --rm by default which totally changes things! In the anka plugin, we clone the VM and then execute the anka run inside of a for loop; That VM stays up until we're done with it and our pre-exit hook will clean up the clone.

We have a feature which will keep the container around for investigation if commands fail, so we didn't want to forcefully remove the container with anka's --rm (not even sure it has one anyway).

Outside of the better logging and ability to match Linux and Mac instructions/yml, I'm going to have to think about it a bit. Anka and Docker and definitely different beasts, and this isn't a show stopper.

Side note: I remember seeing an issue with [[ -z $VAR_BLAH ]] && DO SOMETHING || [[ -z $VAR2_BLAH ]] && DO SOMETHING ELSE in the command: | and it was having issues; I'll have to double check this cause it's been a while. It could very well have been a different failure and I didn't notice the real reason.

from docker-buildkite-plugin.

toolmantim avatar toolmantim commented on July 28, 2024

Ah yeah, that makes sense with a full VM setup like that. Thanks for the context! 🙏🏼

What if you went with something that mirror'd both the way Buildkite normally handles multi-line command, and mirrors this Docker plugin in having an additional command/commands option that has different semantics than usual.

For example:

steps:
  # Spins up a VM
  # anka run `/bin/sh -e -c "make test"``
  # Teardown
  - command: make test
    plugins:
      - chef/anka#v0.5.1:
          vm-name: macos-base-10.14
steps:
  # Spin up a VM
  # anka run `/bin/sh -e -c "make build\nmake test"`
  # Teardown
  - commands: |
      make build
      make test
    plugins:
      - chef/anka#v0.5.1:
          vm-name: macos-base-10.14
steps:
  # Spin up a VM
  # anka run `/bin/sh -e -c "make test1"`
  # anka run `/bin/sh -e -c "make test2"`
  # Teardown
  - plugins:
      - chef/anka#v0.5.1:
          vm-name: macos-base-10.14
          commands: |
            make test1
            make test2

And/or works as an array:

steps:
  # Spin up a VM
  # anka run `/bin/sh -e -c "make test1"`
  # anka run `/bin/sh -e -c "make test2"`
  # Teardown
  - plugins:
      - chef/anka#v0.5.1:
          vm-name: macos-base-10.14
          commands:
            - make test1
            - make test2

from docker-buildkite-plugin.

lox avatar lox commented on July 28, 2024

My take is that the docker and docker-compose plugins try and match how the underlying tools work as much as possible. For docker and docker-compose, that means each run would be in it's own container. That would end up very confusing for users for the examples @toolmantim posted in #126 (comment).

Anka is different, it runs commands in a running VM. It makes sense that the plugin works like it does in that case.

In Docker's world, the closest parallel would be docker exec against a detached container, which would make a lovely plugin on it's own, but not as part of this plugin, IMO.

from docker-buildkite-plugin.

toolmantim avatar toolmantim commented on July 28, 2024

Thanks for the input @lox!

Let's close this for now, but thanks for bringing it up @NorseGaud. We don't have many of these design constraints / considerations written down for plugin authors, so it's awesome to hear about what design challenges you're hitting. Keep up the feedback 🙌🏼❤️

from docker-buildkite-plugin.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.