Skip to content

added Dockerfile and service-json file in flo-executor and flo-server#33

Open
sugandhpasricha wants to merge 8 commits into
Swiggy:internal-releasefrom
sugandhpasricha:rock-compatibility
Open

added Dockerfile and service-json file in flo-executor and flo-server#33
sugandhpasricha wants to merge 8 commits into
Swiggy:internal-releasefrom
sugandhpasricha:rock-compatibility

Conversation

@sugandhpasricha
Copy link
Copy Markdown

  1. Dockerfile has been added to containerise conductor based services flo-server and flo-executor.
  2. flo-server directory is added which contains service-json file for the flo-server service
  3. flo-executor directory has been added which contains service-json file for flo-executor service

"memory": 2048,
"scalingDefinition": [
{
"minReplicas": 1,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean? If cpu is more then 75% still we will use max replicas 1?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This can be changed according to the requirement of the service.
This is the default setting of the service-template file.

"spec": [
{
"type": "container",
"name": "",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we are giving empty string in a name?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these are the default setting. It can be changed as per service requirement.

"healthPort": 8080,
"healthUrl": "/api/health",
"metricPort": 8080,
"accessLevel": "swiggy",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wanted to understand accessLevel swiggy wide.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set application endpoint accessibility (internal-vpc/peered vpc, swiggy- From swiggy corporate office)

Comment thread flo-executor/service-template.json Outdated
{
"name": "flo-executor",
"pod": "flo-executor",
"version": "0.0.3",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When do we need to bump up the version? Also why it starts with 0.0.3?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any changes in the image or serivce-template.json file will lead to version incrementation.

Comment thread Dockerfile Outdated
@@ -0,0 +1,28 @@
{
"name": "flo-executor",
"pod": "flo-executor",
Copy link
Copy Markdown

@naveenchlsn naveenchlsn Oct 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shoudnt the pod be ff ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you elaborate why it should be ff? We are following default setting wherein the pod name and service name is same if not mentioned otherwise.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ff is the pod name for conductor. Are you sure that we must set pod name as service name. If so can you give me some examples of other services ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so far we are using same pod name and service name in "name" and "pod" sections as default setting unless the service is already deployed in rock using a different "pod". If the pod already exists for a particular service coast will not deploy the service in a pod with a different name, it gives an error.

Comment thread flo-executor/service-template.json Outdated
Comment thread Dockerfile
FROM gradle:4.8.1 AS build
COPY --chown=gradle:gradle . /home/gradle/conductor
WORKDIR /home/gradle/conductor
RUN gradle build -x test
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gradle build will take time to execute which will increase our docker deployment time. can you please setup so that the jar is directly picked up from swiggy dockerhub.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dockerfile structure has already been discussed keeping best practices in consideration.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@naveenchlsn you can consult this url https://github.com/Netflix/conductor/blob/master/docker/server/Dockerfile , this dockerfile also contains gradle build which comes under best practices

Comment thread flo-executor/service-template.json Outdated
{
"type": "container",
"name": "",
"image": "hashedin/conductor-server:0.0.1",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider removing hashedin from here and put ff the pod name

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hashedin is a test repo created in harbor as well as test env created in coast.Image can be push in a different repo as well once it is indicated to be done that way.

@@ -0,0 +1,28 @@
{
"name": "flo-executor",
"pod": "flo-executor",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ff is the pod name for conductor. Are you sure that we must set pod name as service name. If so can you give me some examples of other services ?

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 this pull request may close these issues.

3 participants