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

Implement CI/CD pipeline #128

Open
jviitamo opened this issue Jan 15, 2022 · 5 comments · May be fixed by #139
Open

Implement CI/CD pipeline #128

jviitamo opened this issue Jan 15, 2022 · 5 comments · May be fixed by #139

Comments

@jviitamo
Copy link
Contributor

One major improvement to Ilmomasiina could be to add CI/CD pipeline to ensure code stability before merging changes into production. One alternative to testing could be Cypress.

@Sonlis
Copy link
Contributor

Sonlis commented Jan 16, 2022

@jviitamo How's the deployment process currently ? From my limited knowledge, someone has to run manually a command to deploy on Otax, do we want to automate this on merges to otax/production, and force PR to have your review before being merged ?

@jviitamo
Copy link
Contributor Author

You are totally correct about the deployment process. I am not sure if we want to automate the deployment to otax, is that a possible security issue @samporapeli? If not, the way you described would be perfect! I think that merging rights should also be limited then to the CTO only. To start with, we could make some tests to make sure code is valid even before making merging possible.

@samporapeli
Copy link
Member

Well, that would be a way to run code at otax without an ssh key, but I don't think it's a reason to not develop such pipeline, should it be considered an important or useful feature. I think the deployment process is simple enough already and it's also not completely bad thing to have more control over when to update, for example, code can be merged during day and deployed at night when there's no or little usage. Or if a change requires modifications to .env configuration file, it may be nice that the change isn't automatically pushed to production, potentially breaking it if configuration isn't yet changed.

I think it would be fine if only CTO and/or selected persons are able to merge into main branch, but it would be convenient that hacker crew still has commit rights to other branches as opposed to developing in own forks. I am not sure how Github's access control works and if that's possible.

One possible deployment technique could be that a bare git repository is added to otax, that has post push hooks that deploy the new code. That way deployment is limited to persons with a ssh key, and code can be updated to Github and otax at different times.

@Sonlis
Copy link
Contributor

Sonlis commented Jan 18, 2022

IIRC it is absolutely possible to limit branch merges to certain people, while leaving commits / pushes to other branches.
I was asking about the CD since the issue mentions CI and CD, and I think they should be broken into 2 different problems. CI is pretty straight forward, just have to figure which tools to use. However CD requires us to define what we want. Do we want everything to be automatized ? Or do we want more control over the timing of deployments ?

IMO currently features / bug fixes are being implemented at quite reasonable time, so I don't think it's such a burden to the CTO to run manually the steps to deploy. Which is why I agree with the deployment technique proposed by @samporapeli.

@jviitamo
Copy link
Contributor Author

Sampo's proposal sounds good. Maybe it would have been a better idea to make two different issues for CI and CD, as one is needed more than the other. In my opinion, the deployment process is less important to automatize than the CI. Consequently, I'd rather first start making the CI and only continue with CD after it.

@nikosavola nikosavola linked a pull request Feb 23, 2022 that will close this issue
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

Successfully merging a pull request may close this issue.

3 participants