Skip to content
This repository has been archived by the owner on Nov 16, 2020. It is now read-only.

[CI] Build pipeline #6

Open
4 tasks
haveyaseen opened this issue Apr 19, 2020 · 15 comments · Fixed by #9
Open
4 tasks

[CI] Build pipeline #6

haveyaseen opened this issue Apr 19, 2020 · 15 comments · Fixed by #9
Labels
devops Developer operations: CI, deployment etc. good first issue Good for newcomers help wanted Extra attention is needed

Comments

@haveyaseen
Copy link
Member

haveyaseen commented Apr 19, 2020

Add GitHub Actions workflow.

  • Disable/Remove Python workflow
  • Build Go code
  • Run Postgres in Docker
  • Run tests
@haveyaseen haveyaseen added good first issue Good for newcomers help wanted Extra attention is needed labels Apr 19, 2020
@haveyaseen haveyaseen changed the title [CI] Build Go code [CI] Build pipeline Apr 19, 2020
@Addono
Copy link
Contributor

Addono commented Apr 20, 2020

Any pointers on the commands needed to setup the environment and to run the tests?

@haveyaseen
Copy link
Member Author

@Addono As far as I know:

  • Install golang through package manager (or start from a golang docker image)
  • Start PostgreSQL docker container
  • Running tests: go test -v github.com/ito-org/go-backend
  • Running the server: go run github.com/ito-org/go-backend

cc @calmandniceperson

@rettetdemdativ
Copy link
Contributor

@Addono For both testing and running the backend application you need to have Postgres running. I'm going to make some changes so you can spin up the database without prior configuration.

Once Postgres is up and running, we need go test github.com/ito-org/go-backend to work. Deployment (I wouldn't know where to deploy it currently) would work through go install github.com/ito-org/go-backend.

@Addono
Copy link
Contributor

Addono commented Apr 20, 2020

Deployment seems like a separate issue to me, which should also be included in CI but not defined by it.

Anyway, I would suggest containerizing it or trying to run it on serverless (e.g. on Zeit Now or AWS using the Serverless Framework).

Serverless is more experimental and probably requires some changes in the code, on the plus side once it's running "it just works" and the only thing you need to worry about is paying the bill once you exceed the free-tier.

Containerizing and running containers will require more configuration and managing infrastructure, hence making it easier to misconfigure and as such suffer down time. However, this option definitely has more flexibility, e.g. adding rate limiting (#4) on your reverse proxy.

If you just need an MVP, then you could always spin up a VM and manually install everything. However, this "pet server" is not future proof, as it doesn't scale and won't be able to recover by itself.

@Addono Addono mentioned this issue Apr 20, 2020
3 tasks
@Addono
Copy link
Contributor

Addono commented Apr 20, 2020

Currently #8 is blocking progress.

@rettetdemdativ
Copy link
Contributor

@Addono Could you try basing your branch on the dev branch, please?

@rettetdemdativ
Copy link
Contributor

Also, when supplying variables such as POSTGRES_PASSWORD to the Postgres container, you can supply these credentials to the backend application through the environment variables POSTGRES_PASSWORD, POSTGRES_USER, and POSTGRES_DB.

@Addono
Copy link
Contributor

Addono commented Apr 20, 2020

Yeah, I noticed. The next thing which breaks is caused by having a hard requirement on a .env file, for one because it feels like an anti-pattern where you need to supply an (empty) file if you want to set them using the environment. More importantly, it breaks the docker-compose up flow, as the .env is not included after building the image (only the build artifact is copied over).

For now this worked:
image

@rettetdemdativ
Copy link
Contributor

rettetdemdativ commented Apr 20, 2020

The most recent commits to the dev branch changed that. Removing it from the docker-compose file is the next step.

@Addono
Copy link
Contributor

Addono commented Apr 20, 2020

So, I am getting some results. The tests fail, however the error message is not really helpful for my eyes.
https://github.com/Addono/api-backend/runs/602239783?check_suite_focus=true

image
image

Are the tests supposed to pass now? The version is based on a recently rebased version from dev.

@MyDigitalLife
Copy link
Contributor

You should also add the gosec security scanner and run a linter. You can add these jobs:

      - name: Run Gosec Security Scanner
        uses: securego/gosec@master
        with:
          args: ./...
      - name: Test
        run: docker run --rm -v $(pwd):/app -w /app golangci/golangci-lint:v1.24.0 golangci-lint run -v

@Dionysusnu
Copy link

@Addono What it's saying is: The test expected a http status code 200, but got a 500. This probably means the endpoint handler errored.

@Addono
Copy link
Contributor

Addono commented Apr 20, 2020

Yeah, looked into the DB logs, which needed to be initialized.

On my fork the tests are now passing, however it's based on the dev branch. How should I PR? Including or excluding the commits on dev?

@Addono
Copy link
Contributor

Addono commented Apr 20, 2020

You should also add the gosec security scanner and run a linter. You can add these jobs:

      - name: Run Gosec Security Scanner
        uses: securego/gosec@master
        with:
          args: ./...
      - name: Test
        run: docker run --rm -v $(pwd):/app -w /app golangci/golangci-lint:v1.24.0 golangci-lint run -v

I added them as separate jobs, however they both fail.
image
https://github.com/Addono/api-backend/runs/603000917?check_suite_focus=true

@MyDigitalLife
Copy link
Contributor

I fixed most of those issue in a different branch #10

@haveyaseen haveyaseen added the devops Developer operations: CI, deployment etc. label Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
devops Developer operations: CI, deployment etc. good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants