Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: qltysh/qlty
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: main
Choose a base ref
...
head repository: codefreak/codeclimate
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: master
Choose a head ref
Can’t automatically merge. Don’t worry, you can still create the pull request.
  • 6 commits
  • 5 files changed
  • 2 contributors

Commits on Aug 24, 2019

  1. Permit mounting via volumes-from by passing orchestrator ID

    tl;dr This helps CodeClimate engines not need intimiate docker host
    knowledge.
    
    In contexts like self-hosted Gitlab, we sometimes have a context where
    we have an invoking runner like Gitlab CI running the Docker executor,
    which exposes the Docker socket to the running job, so that the running
    job may invoke its own Docker jobs on the host. Gitlab's top-level job
    will set up some filesystem context (/builds, mounted as a Docker
    volume, in the Gitlab case).
    
    Right now, Gitlab can only support CodeClimate in a Docker-in-Docker
    runner, because CodeClimate performs volume mounting for the individual
    engines via Docker's --volume flag, which mounts not the path from the
    invoking container, but rather a path on the docker host. This requires
    that the path passed to CodeClimate as the CODECLIMATE_CODE variable
    match the real host path, and in the Gitlab CI case, we don't want that,
    so we have to "hide" the host with a DinD approach. However, this means
    that we also don't get any layer caching between jobs, which makes
    running CodeClimate prohibitively expensive, as all the engines etc have
    to be downloaded for each job.
    
    By supporting Docker's `volumes-from` mounting option, we can instead
    tell the engines to inherit any mounts from the invoking orchestrator.
    This permits CodeClimate to allow the top-level context set up a Docker
    volume, bind it to the orchestrator, and then allow the orchestrator to
    pass that to invoked children. This sidesteps the issue of the Engines
    needing to know the actual host path; as long as the orchestrator's
    /code directory is mounted, the children can just presume to use it
    as-is.
    
    To accomplish this, we just a) name the top-level container, and b) pass
    that name via the CODECLIMATE_ORCHESTRATOR env var:
    
            docker run \
              --interactive --tty --rm \
              --name codeclimate_orchestrator \
              --env CODECLIMATE_ORCHESTRATOR="codeclimate_orchestrator" \
              --env CODECLIMATE_CODE="/code" \
              --volume "$PWD":/code \
              --volume /var/run/docker.sock:/var/run/docker.sock \
              --volume /tmp/cc:/tmp/cc \
              codeclimate/codeclimate-wrapped analyze
    
    In the bare-metal case, this doesn't change anything - we're mounting
    the real host path, which then gets passed to the individual children
    mounted on the /code mount.
    
    While not immediately pertinent to the CodeClimate PR, In Gitlab, we can
    invoke the Gitlab codequality image like so:
    
        script:
            - CONTAINER_ID=$(docker ps -q -f "label=com.gitlab.gitlab-runner.job.id=${CI_JOB_ID}")
            - BUILDS_VOLUME_ID=$(docker inspect $CONTAINER_ID --format '{{ range .Mounts }}{{ if eq .Destination "/builds" }}{{ .Name }}{{ end }}{{ end }}')
            - docker run
                --rm
                --name "codeclimate_orchestrator_${CI_JOB_ID}"
                --env SOURCE_CODE="/code"
                --env CODECLIMATE_VERSION="volumes-from"
                --env ORCHESTRATOR_ID="codeclimate_orchestrator_${CI_JOB_ID}"
                --volume /var/run/docker.sock:/var/run/docker.sock
                --volume "${BUILDS_VOLUME_ID}":/code
                codequality:orch /code
    
    ("volumes-from" is my local Docker image for the altered CodeClimage
    build, and "codequality:orch" is my altered Gitlab codequality image)
    
    Because this job _must_ be executed in a context that is visible to
    Docker, we can query Docker to get the current job's container ID, and
    from there get the volume ID mounted as `/builds`. We then volume mount
    that volume as /code, and specify /code as the "host" location of our
    code to be evaluated. The orchestrator will use the passed volume as
    /code, which is then passed onto the engine jobs, allowing the entire
    process to run against an ephemeral Docker volume rather than requiring
    a known path on the host.
    cheald committed Aug 24, 2019
    Copy the full SHA
    242e353 View commit details

Commits on Aug 26, 2019

  1. Copy the full SHA
    c1c0b00 View commit details

Commits on Sep 18, 2019

  1. Copy the full SHA
    5ef15bf View commit details

Commits on Nov 19, 2019

  1. Add custom linting engine

    HenningCash committed Nov 19, 2019
    Copy the full SHA
    2da8d46 View commit details
  2. Disable update check

    HenningCash committed Nov 19, 2019
    Copy the full SHA
    8c45f42 View commit details
  3. Copy the full SHA
    5bded17 View commit details
Showing with 32 additions and 4 deletions.
  1. +1 −1 Dockerfile
  2. +1 −0 config.yml
  3. +4 −0 config/engines.yml
  4. +13 −3 lib/cc/analyzer/engine.rb
  5. +13 −0 spec/cc/analyzer/engine_spec.rb
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@ RUN wget -q -O /tmp/docker.tgz \
rm -rf /tmp/docker*

COPY . /usr/src/app

COPY config.yml /config.yml
VOLUME /code
WORKDIR /code
ENTRYPOINT ["/usr/src/app/bin/codeclimate"]
1 change: 1 addition & 0 deletions config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
check-version: false
4 changes: 4 additions & 0 deletions config/engines.yml
Original file line number Diff line number Diff line change
@@ -275,3 +275,7 @@ vint:
channels:
stable: codeclimate/codeclimate-vint
description: Fast and Highly Extensible Vim script Language Lint implemented by Python.
cpplint:
channels:
stable: cfreak/codeclimate-cpplint
description: Cpplint is a command-line tool to check C/C++ files for style issues following Google's C++ style guide.
16 changes: 13 additions & 3 deletions lib/cc/analyzer/engine.rb
Original file line number Diff line number Diff line change
@@ -81,16 +81,26 @@ def container_options
"--memory-swap", "-1",
"--net", "none",
"--rm",
"--volume", "#{code.host_path}:/code:ro",
"--volume", "#{config_file.host_path}:/config.json:ro",
"--user", "9000:9000"
"--user", "9000:9000",
"--volume", "#{config_file.host_path}:/config.json:ro"
]

if orchestrator_id && orchestrator_id != ""
options.concat(["--volumes-from", "#{orchestrator_id}:ro"])
else
options.concat(["--volume", "#{code.host_path}:/code:ro"])
end

if (memory = metadata["memory"]).present?
options.concat(["--memory", memory.to_s])
end
options
end

def orchestrator_id
ENV["CODECLIMATE_ORCHESTRATOR"]
end

def container_name
@container_name ||= "cc-engines-#{qualified_name.tr(":", "-")}-#{SecureRandom.uuid}"
end
13 changes: 13 additions & 0 deletions spec/cc/analyzer/engine_spec.rb
Original file line number Diff line number Diff line change
@@ -39,6 +39,19 @@ module CC::Analyzer
engine.run(StringIO.new)
end

it "runs a Container mounting volumes from an orchestrator" do
container = double
allow(container).to receive(:on_output).and_yield("")
expect(container).to receive(:run).with(including(
"--volumes-from", "codeclimate_orchestrator:ro"
)).and_return(Container::Result.new)

expect(Container).to receive(:new).and_return(container)
engine = Engine.new("", metadata, {}, "a-label")
allow(engine).to receive(:orchestrator_id).and_return("codeclimate_orchestrator")
engine.run(StringIO.new)
end

it "runs a Container with engine memory overrides" do
container = double
allow(container).to receive(:on_output).and_yield("")