Skip to content

Commit

Permalink
Failing when port is specified with labels
Browse files Browse the repository at this point in the history
Resolves #522
Kompose will give FATAL error if labels are given but ports are not defined
  • Loading branch information
surajnarwade committed Jun 14, 2017
1 parent 3be76ff commit 642406a
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 0 deletions.
3 changes: 3 additions & 0 deletions docs/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ The currently supported options are:
| kompose.service.type | nodeport / clusterip / loadbalancer |
| kompose.service.expose| true / hostname |
**Note**: `kompose.service.type` label should be defined with `ports` only, otherwise `kompose` will fail.

## Restart

Expand Down Expand Up @@ -401,3 +402,5 @@ If the Docker Compose file has a volume specified for a service, the Deployment
If the Docker Compose file has service name with `_` in it (eg.`web_service`), then it will be replaced by `-` and the service name will be renamed accordingly (eg.`web-service`). Kompose does this because "Kubernetes" doesn't allow `_` in object name.

Please note that changing service name might break some `docker-compose` files.


24 changes: 24 additions & 0 deletions pkg/loader/compose/compose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,3 +323,27 @@ func TestNormalizeServiceNames(t *testing.T) {
}
}
}

func TestCheckLabelsPorts(t *testing.T) {
testCases := []struct {
name string
noOfPort int
labels string
svcName string
expectError bool
}{
{"ports is defined", 1, "NodePort", "foo", false},
{"ports is not defined", 0, "NodePort", "foo", true},
}

var err error
for _, testcase := range testCases {
t.Log(testcase.name)
err = checkLabelsPorts(testcase.noOfPort, testcase.labels, testcase.svcName)
if testcase.expectError && err == nil {
t.Log("Expected error, got ", err)

}

}
}
11 changes: 11 additions & 0 deletions pkg/loader/compose/v1v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,10 @@ func libComposeToKomposeMapping(composeObject *project.Project) (kobject.Kompose
serviceConfig.ExposeService = strings.ToLower(value)
}
}
err = checkLabelsPorts(len(serviceConfig.Port), composeServiceConfig.Labels["kompose.service.type"], name)
if err != nil {
return kobject.KomposeObject{}, errors.Wrap(err, "kompose.service.type can't be set if service doesn't expose any ports.")
}

// convert compose labels to annotations
serviceConfig.Annotations = map[string]string(composeServiceConfig.Labels)
Expand All @@ -266,3 +270,10 @@ func libComposeToKomposeMapping(composeObject *project.Project) (kobject.Kompose
}
return komposeObject, nil
}

func checkLabelsPorts(noOfPort int, labels string, svcName string) error {
if noOfPort == 0 && labels == "NodePort" || labels == "LoadBalancer" {
return errors.Errorf("%s defined in service %s with no ports present. Issues may occur when bringing up artifacts.", labels, svcName)
}
return nil
}
4 changes: 4 additions & 0 deletions script/test/cmd/tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ convert::expect_success_and_warning "kompose --provider openshift -f $KOMPOSE_RO
# Test regarding validating dockerfilepath
convert::expect_failure "kompose -f $KOMPOSE_ROOT/script/test/fixtures/dockerfilepath/docker-compose.yml convert --stdout"

# Test regarding while label (nodeport or loadbalancer ) is provided but not ports
convert::expect_failure "kompose -f $KOMPOSE_ROOT/script/test/fixtures/label-port/docker-compose.yml convert --stdout"


######
# Test the output file behavior of kompose convert
# Default behavior without -o
Expand Down
6 changes: 6 additions & 0 deletions script/test/fixtures/label-port/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
version: "2"
services:
webapiapplication:
image: webapiapplication
labels:
kompose.service.type: NodePort

0 comments on commit 642406a

Please sign in to comment.