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

[WIP] Network Mounted Volumes - docker based solution #224

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

angad-k
Copy link
Member

@angad-k angad-k commented Oct 25, 2020

Disregard the previous PR. The work for this is mostly completed. Once the next seaweedFS release happens (which usually does every 5-6 days), I'll rebuild the docker images for the databases and then include those images instead of the current ones we are using.

dockerozed seaweedfs, doesn't change any database behaviour as of now

no breaking changes
@angad-k angad-k changed the title [WIP} Network Mounted Volumes - docker based solution [WIP] Network Mounted Volumes - docker based solution Oct 25, 2020
Copy link
Contributor

@karan0299 karan0299 left a comment

Choose a reason for hiding this comment

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

Few requested changes
Rest L.G.T.M

@@ -98,3 +99,44 @@ func setupDatabaseContainer(serviceName string) {
}
}
}

func setupSeaweedfsContainer(serviceName string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In this function change serviceName -> seaweedType

utils.LogInfo("No %s instance found in host. Building the instance.", strings.Title(serviceName))
containerID, err := seaweedfs.SetupSeaweedfsInstance(serviceName)
if err != nil {
utils.Log("There was a problem deploying %s service.", strings.Title(serviceName), utils.ErrorTAG)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
utils.Log("There was a problem deploying %s service.", strings.Title(serviceName), utils.ErrorTAG)
utils.Log("There was a problem deploying %s seaweedType", strings.Title(serviceName), utils.ErrorTAG)

if containerCfg.Name == types.SeaweedFiler {
err := os.MkdirAll("seaweed/seaweed-filer-storage/filerldb2", 0777)
if err != nil {
println(err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't here be return statement instead of print

},
PortBindings: nat.PortMap{
nat.Port(containerPortRule1): []nat.PortBinding{{
HostIP: "",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
HostIP: "",
HostIP: "0.0.0.0",

HostIP: "",
HostPort: fmt.Sprintf("%d", containerCfg.ContainerPort1)}},
nat.Port(containerPortRule2): []nat.PortBinding{{
HostIP: "",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
HostIP: "",
HostIP: "0.0.0.0",

@@ -92,6 +92,18 @@ func startAppMakerService() error {
}

func startMasterService() error {

checkAndPullImages("chrislusf/seaweedfs")
err := os.MkdirAll("seaweed/seaweed-filer-storage/filerldb2", 0777)
Copy link
Contributor

Choose a reason for hiding this comment

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

What does ldb2 signify in filerldb2 ? and Is it necessary to name it so

//SeaweedCronjob is the cronjob service for Seaweedfs
SeaweedCronjob = "seaweed_cronjob"

//SeaweedS3 is the Seaweed service that provides support for AmazonS3
Copy link
Contributor

Choose a reason for hiding this comment

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

@supra08 Is support for AmazonS3 required for our use case? In future may be?

Image string
// Port on which a database service is running inside the container
HostPort1 int
// Port on which a database service is running inside the container
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change Comment to distinguish HostPort1 and HostPort2

ContainerPort1 int
// Port of the docker container in the host system
ContainerPort2 int
// Directory inside the docker container for volume mounting purposes
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly for ContainerPort1 and ContainerPort2

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.

2 participants