Refactoring: Support of different deployments kind #30
Conversation
internal/app/upgradeagent/objects.go
Outdated
| @@ -4,6 +4,19 @@ import ( | |||
| "time" | |||
| ) | |||
|
|
|||
| // DeploymentClient is a common interface for any deployment kinds. | |||
| type DeploymentClient interface { | |||
| UpgradeCheck(LocalSolutionInfo) bool | |||
There was a problem hiding this comment.
Would suggest that the names of functions be imperative, like DoSomething.
CheckUpgrade
CheckHealth
There was a problem hiding this comment.
ok. I'll do changes.
| @@ -0,0 +1,36 @@ | |||
| package upgradeagent | |||
|
|
|||
There was a problem hiding this comment.
Would suggest file names in snake case.
docker_compose_deployment.go
There was a problem hiding this comment.
ok, I'll rename the files.
| @@ -0,0 +1,36 @@ | |||
| package upgradeagent | |||
|
|
|||
There was a problem hiding this comment.
Would suggest file names in snake case.
kubernetes_deployment.go
| @@ -4,6 +4,19 @@ import ( | |||
| "time" | |||
| ) | |||
|
|
|||
| // DeploymentClient is a common interface for any deployment kinds. | |||
| type DeploymentClient interface { | |||
There was a problem hiding this comment.
Whose client is this interface? Which is the server?
There was a problem hiding this comment.
The client of this interface is the Patrao agent. It's not implemented yet. I'm working on it now. I suppose Today evening or Monday morning I'll commit these changes.
| } | ||
|
|
||
| // UpstreamClient is a common interface for any upstream service kinds | ||
| type UpstreamClient interface { |
There was a problem hiding this comment.
Whose client is this interface? Which is the server? Who uses this interface / calls its methods?
There was a problem hiding this comment.
The client of this interface is DeploymentClient. We pass UpstreamClient while creation of DeploymentClient instance and then created instance calls RequestUpgrade() functions. For example, please see func (d *dockerComposeDeployment) UpgradeCheck(localSolutionInfo LocalSolutionInfo) bool implementation. As for the server (ip:port) it will be received from the command line arguments inside the implementation of mockUpstreamClient. Actually in the same way how it's working now.
| continue | ||
| } | ||
| val, exist := containersSpec[serviceName] | ||
| if (exist) && (upgradeInfo.Name == solutionName) { |
There was a problem hiding this comment.
You can avoid the parentheses.
if val, exist := containersSpec[serviceName]; exist && upgradeInfo.Name == solutionName {
...
}
|
|
||
| rc = false | ||
| containersSpec := make(map[string]ContainerSpec) | ||
| specMap := make(map[string]interface{}) |
There was a problem hiding this comment.
Since you're unmarshaling into this map, you don't need to to preallocate. More idiomatic:
var specMap map[string]interface{}
if err := yaml.Unmarshal([]byte(upgradeInfo.Spec), &specMap); err != nil {
...
There was a problem hiding this comment.
Since the map variable is a reference type like pointer it has null. So we have to preallocate the map before passing it to yaml.Unmarshal
| } | ||
| if found { | ||
| containersSpec[service.(string)] = ContainerSpec{Name: service.(string), Image: serviceImageName} | ||
| } |
There was a problem hiding this comment.
Can be simplified:
if val, itemFound := details[DockerComposeImageName]; itemFound {
if serviceImageName, ok := val.(string); ok {
containersSpec[service.(string)] = ContainerSpec{Name: service.(string), Image: serviceImageName}
}
}
| return false | ||
| } | ||
| if val, exists := specMap[DockerComposeServicesName]; exists { | ||
| servicesMap := val.(map[interface{}]interface{}) |
There was a problem hiding this comment.
Will the following work? Might be slightly simpler code. Plus we must check for ok:
servicesMap, ok := val.(map[string]interface{})
if !ok {
continue
}
| } | ||
| err := os.Mkdir(d.upgradeInfo.Name, os.ModePerm) | ||
| if nil != err { | ||
| log.Error(err) |
There was a problem hiding this comment.
You might want to add meaningful context info to err before logging and returning.
| if err != nil { | ||
| log.Error(err) | ||
| return err | ||
| } |
There was a problem hiding this comment.
if err = cmd.Run(); err != nil {
...
}
| return true | ||
| } | ||
| time.Sleep(1 * time.Second) | ||
| } |
There was a problem hiding this comment.
You can reduce indentation be removing these {...}.
Resolves #29