Brigade: Can't use brigadier as lib for custom pipelining libraries.

Created on 9 Feb 2018 · 13Comments · Source: brigadecore/brigade

Hi!

This is very related with #277 and #240

At this moment we are setting up brigade and one of the requirements is to provide a set of utilities to create pipelines to the different teams. After reading some of the code (I'm new to the js/ts world, my day to day is more related with Go, infrastructure and backend) and making a lot of things that I'm not proud of, it didn't work.

I'll start explaining what our library tries to do (maybe this is not something a brigade pipeline custom code should do and there is the problem itself)

For example our library does this kind of things:

import * as brigadier from "./brigadier"

const ghStatusImage = "technosophos/github-notify:latest"

/**
 * BuildStatus is the status of a build.
 */
export enum BuildStatus {
    Pending = "pending",
    Failure = "failure",
    Error = "error",
    Success = "success",
}

/**
 *
 * NotificationJobGenerator will return different notification jobs for different kind of targets.
 */
export class NotificationJobGenerator {
    constructor(public event: brigadier.BrigadeEvent, public project: brigadier.Project){}

    /**
     * githubStatus will set on the specific commit of the build the status.
     * 
     * @param status is the status that will be set.
     */
    public githubStatus(status: BuildStatus): brigadier.JobAbs{
        let job = new brigadier.Job("set-github-build-status", ghStatusImage)
        job.env = {
            GH_REPO: this.project.repo.name,
            GH_STATE: status,
            GH_DESCRIPTION: `Brigade build finished with ${this.event.cause.trigger} state`,
            GH_CONTEXT: "brigade",
            GH_TOKEN: this.project.repo.token,
            GH_COMMIT: this.event.commit
        }
        return job;
    }
}

As you see I have a local import that says:

import * as brigadier from "./brigadier"

I need to do this because importing the brigadier module tries to run brigadier as an app.

So I have an ugly/hack file called brigadier.ts

// WARNING: This is an ugly hack done because we can't export brigadier as a package because it will run the app.
// Until brigadier can be use as a proper library or module we need to import only the files that we want.
// This should be removed ASAP.
export { BrigadeEvent, Project } from "../node_modules/brigadier/dist/events";
export { Job as JobAbs } from "../node_modules/brigadier/dist/job";
export { Job } from "../node_modules/brigadier/dist/brigadier";

Yes its ugly and bad :( but I don't know another way of loading brigadier without it tries to run the app.

After this, it the job is created perfectly but it fails because the job creates a JobRunner and the job runner needs some module scoped variables that are set by fire() function.

[brigade:app]   Error: TypeError: Cannot read propert  y 'commit' of null

    - k8s.js:115 n  ew JobRunner
      [src]/[brigade-spotahome]/[brigadier]/dist/k8s.js:115:24  [0m

    - brigadier.js:50   [37mJob.run
      [src]/[brigade-spotahome]/[brigadier]/dist/brigadier.js:5  0:18

    - brigade.js:108   37mfullBuild
      ./dist/brigade.js:108:18

    - brigade.js:121   37mEventRegistry.debugFullBuild
      ./dist/brigade.js:121:5

    - events.js:75   mEventRegistry.fire
      ./dist/events.js:75:14

    - brigadier.js:34   [37mObject.fire
      ./dist/brigadier.js:34:20

    - app.js:137 l  oadProject.then.then
      ./dist/app.js:137:23

This error is correct as our code uses brigadier code as a library and is in a different scope that the one that the worker loads to run brigade.js script.

As I see in brigadier.ts JobRunner instance creation takes module scoped variables.

let currentEvent = null;
let currentProject = null;
...
export function fire(e: eventsImpl.BrigadeEvent, p: eventsImpl.Project) {
  currentEvent = e;
  currentProject = p;
  events.fire(e, p);
}

....
export class Job extends jobImpl.Job {
  run(): Promise<jobImpl.Result> {
    let jr = new JobRunner(this, currentEvent, currentProject);
    this._podName = jr.name;
    return jr.run().catch(err => {
      // Wrap the message to give clear context.
      console.error(err)
      let msg = `job ${this.name}(${jr.name}): ${err}`;
      return Promise.reject(new Error(msg));
    });
  }
}

I could extend the Job abstract class but Job runner also uses module/global scope variables and in the end I would have the same problem in a different place.

Maybe I haven't understood correctly how I could create a custom library that uses brigade, or also as a new "Typescripter" I'm doing lots and lots of wrong things :)

As a note I would like to avoid setting our code as part of brigadier and not use ./mylib (relative imports of our code) in the brigade.js pipeline files. I guess this would work perfect because brigadier scope is shared with our code (in the end it's the same).

Thank you!.

question

Source

slok

👍2

All 13 comments

I'm looking at this one pretty closely. What do you mean when you say:

importing the brigadier module tries to run brigadier as an app.

If you return a brigadier.Job instead of a brigadier.JobAbs from githubStatus(), doesn't that allow the fire() event to appropriately set the module-scoped vars?

I'm going to hack around on this and see if I can find an elegant way to do this.

technosophos on 12 Feb 2018

So in playing around with this idea, I have it a try here: https://github.com/technosophos/test-extend-brigade

That project adds a single extra library that does part of what you do above (it returns a pre-build Job). Does that get you any further?

technosophos on 12 Feb 2018

Thanks for taking your time to read and test a solution!

I'm looking at this one pretty closely. What do you mean when you say:

importing the brigadier module tries to run brigadier a an app.

If my library has brigadier as a dependency. I would do this.

import * as brigadier from "brigadier"

This triggers index.ts import from brigadier, that has:

// Run the app.
new App(projectID, projectNamespace).run(e);

so the import of anything in that way, executes this, for example when I try to do a new Job(...).

If you return a brigadier.Job instead of a brigadier.JobAbs from githubStatus(), doesn't that allow the fire() event to appropriately set the module-scoped vars?

The problem here is that I have brigadier as an installed dependency of our library, this means that we don't have the context to populate with or without fire, that could be solved passing stuff as arguments to the constructors or methods (in other words dependency injection).

So in playing around with this idea, I have it a try here: https://github.com/technosophos/test-extend-brigade

That project adds a single extra library that does part of what you do above (it returns a pre-build Job). Does that get you any further?

That works! that was what I thought that could work but our first idea was to have a standalone library that you could install with npm, yarn... or upload to npm registry so it can be shared to other people. Doing this custom library as a independent library, needs to take brigadier as a dependency and when you use our library from a brigade.js file, this calls brigadier stuff (like Job), the library gets the code from the dependency(brigadier code in library node_modules), and this dependency doesn't have populated globals.

The approach that you develop will work so we will stick with it :)

Thank you! and sorry... It's difficult to explain this and my english is not the best one neither, so I guess a lot of things are difficult to understand or poorly explained.

slok on 13 Feb 2018

Ah! I see. I had missed that you were creating an NPM module.

I guess the way the library is constructed right now, you'd have to do some sort of dependency injection at startup time. The NPM module case is totally legit. I need to think about how we can make that possible. The main thing we were doing in the brigadier library is inject stuff without the user having to have any knowledge of it. And I can see that the way we implemented that is causing a probably unnecessary hurdle for module development.

I'm totally up for changing the library, provided it doesn't add more steps for the brigade.js developer.

technosophos on 13 Feb 2018

Some new stuff was merged recently that will let you set a custom worker image. This might make it at least a little easier to load custom TS/JS.

I'm still exploring how we can make it easy to do node modules, which really seems to be the ideal way of extending.

technosophos on 15 Feb 2018

❤1

Yeah! all the things that we were testing on the custom brigade lib were using #277 branch. That helped a lot :)

Thanks for all the effort!

slok on 16 Feb 2018

❤1

We now have some docs for this: https://github.com/Azure/brigade/blob/master/docs/topics/workers.md

technosophos on 6 Mar 2018

Awesome! thank you!

We are making improvements to our custom library and try to abstract the deployments and common jobs so the pipeline files of the projects are simple and similar, If you like we could explain our use case in a future when it's more mature :smiley:

slok on 6 Mar 2018

That would be fabulous! I'd love to hear about it.

technosophos on 7 Mar 2018

We're running into this issue, too. Right now we're using the custom worker method, but it's a bit heavy, requires every user who wants to add utility functions to have to push access to the Docker registry etc., and makes it hard for us to develop our own brigade pipelines and contribute to Brigade at the same time (since we're not sure whether to put our containers our yours as the FROM on the custom workers at any given time).

I had a thought of adding to the brigade/brigade chart's values.yaml a section that will let users reference a Javascript file, and have Helm package up the JS file and store it as a configMap or secret to then be mounted on workers as an accessible volume--rendering the package importable via npm automatically by every instance of the worker. This would completely sidestep the need to have custom containers. Would there be interest in that if I were to work on it? We're thinking of doing it internally regardless.

jdotjdot on 25 Mar 2018

@slok I'm also working on a similar thing to automate the CD pipeline using brigade, can you share your thoughts/approach how are you making a library and using it inside brigad.js?

dinesh on 21 May 2018

Hi @dinesh

I took the structure and an example of our library (we will try to make it OSS) so you could see how we are doing. Here is the repository: https://github.com/slok/brigade-custom-lib-example

It's a little bit ugly but is the only way we have found to develop locally (running tests etc...) and making work as a library inside the worker so in the future we only have to modify one file and not the whole library.

You can get the dependencies and build the sources with: