-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/cd #41
Feature/cd #41
Conversation
default: "latest" | ||
branch: | ||
required: false | ||
description: "Branch to pull in instance deployment" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need a branch in addition to the pre-built docker images ? see the comment in deployment script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cf. bellow
required: false | ||
description: "Size in GB of instance" | ||
type: number | ||
default: 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it changes the price we can lower up to 5 GB (currently backend image is ~2GB and frontend ~30MB)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to repeat te default for variables like volume_size, branch, since they are already in cd/terraform/variable.tf
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how is the pricing for volume boot instance.
For defaults values, is more documentation than real defaults values.
required: false | ||
type: string | ||
description: "Flavor of instance" | ||
default: "s1-2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this is a sandbox flavor, I have no knowledge on this, why chose this flavor in particular ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only for test, we can change this flavor
export APP_URL="https://github.com/${ORG}/${APP_NAME}/archive/refs/heads/${APP_BRANCH}.tar.gz" | ||
export USER=$(lsb_release -si | tr [:upper:] [:lower:]) | ||
|
||
cd /home/$USER && mkdir -p ${APP_NAME} && curl -kLs $APP_URL \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I understand well
- we download the entire code of Basegun repo
- for branch ${APP_BRANCH}
why not:
- only download the files we need (Makefile, maybe docker-compose)
- use the branch with tag ${VERSION} ?(if version==latest we try to find which tag it is ?) it seems logical that the Makefile & docker-compose should be issued from same version as the docker images we are going to pull
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we download the entire code of Basegun for not hardcoding download docker-compose and makefile. Then, github prepare the repo archive natively.
For version, yes, it can be better.
- chmod +x /usr/bin/docker-compose | ||
|
||
final_message: "The system is finally up, after $UPTIME seconds" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very neat 👍
OS_USERNAME: ${{ secrets.OS_USERNAME }} | ||
OS_PROJECT_NAME: ${{ secrets.OS_PROJECT_NAME }} | ||
OS_PROJECT_ID: ${{ secrets.OS_PROJECT_ID }} | ||
OS_PASSWORD: ${{ secrets.OS_PASSWORD }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the username/passwd for an ovh account right ? We said we should create a "deployment" user in these secrets rather than my credentials if I remeber correctly ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the services user is created on OVH. Now, we need to add this creds on github secrets.
required: false | ||
description: "Size in GB of instance" | ||
type: number | ||
default: 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to repeat te default for variables like volume_size, branch, since they are already in cd/terraform/variable.tf
?
name = var.image | ||
most_recent = true | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's not too complex (or too costy), we would appreciate have a s3 volume mounted as well. I don't know how that works, I would be happy to discuss about it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's can be a good issue! I don't know how that is possible.
|
||
variable "app" { | ||
type = string | ||
default = "basegun" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's with a capital B, "Basegun"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tar -zxvf - --strip-components=1 -C ${APP_NAME}
fix this issue
@@ -0,0 +1,8 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when is this executed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is show after deployment in CI's logs
Partie CD de la CI/CD
WORK IN PROGRESS: ne pas merger.