Merge pull request #693 from Azure/go_coding_style

Go coding style

Makefile

@@ -18,31 +18,22 @@ prereqs:
 	go get github.com/jteeuwen/go-bindata/...
 	glide install
 
-_build:
+build:
 	go generate -v $(GOFILES)
 	go build -v -ldflags="-X github.com/Azure/acs-engine/cmd.BuildSHA=${VERSION} -X github.com/Azure/acs-engine/cmd.BuildTime=${BUILD}"
 	cd test/acs-engine-test; go build -v
 
-build: prereqs _build
-
-test: prereqs test_fmt
+test: test_fmt
 	go test -v $(GOFILES)
 
-test_fmt: prereqs
-	test -z "$$(gofmt -s -l $(GOFILES) | tee /dev/stderr)"
+.PHONY: test-style
+test-style:
+	@scripts/validate-go.sh
 
-validate-generated: prereqs
+validate-generated:
 	./scripts/validate-generated.sh
 
-fmt:
-	gofmt -s -l -w $(GOFILES)
-
-lint: prereqs
-	go get -u github.com/golang/lint/golint
-	# TODO: fix lint errors, enable linting
-	# golint -set_exit_status
-
-ci: validate-generated build test lint
+ci: prereqs validate-generated build test lint
 
 devenv:
 	./scripts/devenv.sh

docs/developers.md

@@ -0,0 +1,68 @@
+# Developers Guide
+
+This guide explains how to set up your environment for developing on
+acs-engine.
+
+## Prerequisites
+
+- Go 1.6.0 or later
+- Glide 0.12.0 or later
+- kubectl 1.5 or later
+- An Azure account (needed for deploying VMs and Azure infrastructure)
+- Git
+
+## Contribution Guidelines
+
+We welcome contributions. This project has set up some guidelines in
+order to ensure that (a) code quality remains high, (b) the project
+remains consistent, and (c) contributions follow the open source legal
+requirements. Our intent is not to burden contributors, but to build
+elegant and high-quality open source code so that our users will benefit.
+
+Make sure you have read and understood the main CONTRIBUTING guide:
+
+https://github.com/Azure/acs-engine/blob/master/CONTRIBUTING.md
+
+### Structure of the Code
+
+The code for the acs-engine project is organized as follows:
+
+- The individual programs are located in `cmd/`. Code inside of `cmd/`
+  is not designed for library re-use.
+- Shared libraries are stored in `pkg/`.
+- The `tests/` directory contains a number of utility scripts. Most of these
+  are used by the CI/CD pipeline.
+- The `docs/` folder is used for documentation and examples.
+
+Go dependencies are managed with
+[Glide](https://github.com/Masterminds/glide) and stored in the
+`vendor/` directory.
+
+### Git Conventions
+
+We use Git for our version control system. The `master` branch is the
+home of the current development candidate. Releases are tagged.
+
+We accept changes to the code via GitHub Pull Requests (PRs). One
+workflow for doing this is as follows:
+
+1. Use `go get` to clone the acs-engine repository: `go get github.com/Azure/acs-engine`
+2. Fork that repository into your GitHub account
+3. Add your repository as a remote for `$GOPATH/github.com/Azure/acs-engine`
+4. Create a new working branch (`git checkout -b feat/my-feature`) and
+   do your work on that branch.
+5. When you are ready for us to review, push your branch to GitHub, and
+   then open a new pull request with us.
+
+### Go Conventions
+
+We follow the Go coding style standards very closely. Typically, running
+`go fmt` will make your code beautiful for you.
+
+We also typically follow the conventions recommended by `go lint` and
+`gometalinter`. Run `make test-style` to test the style conformance.
+
+Read more:
+
+- Effective Go [introduces formatting](https://golang.org/doc/effective_go.html#formatting).
+- The Go Wiki has a great article on [formatting](https://github.com/golang/go/wiki/CodeReviewComments).

pkg/acsengine/const.go

@@ -30,7 +30,7 @@ const (
 	// DefaultAgentIPAddressCount is the default number of IP addresses per network interface on agents
 	DefaultAgentIPAddressCount = 1
 	// DefaultAgentMultiIPAddressCount is the default number of IP addresses per network interface on agents,
-	// when VNET integration is enabled. It can be overriden per pool by setting the pool's IPAdddressCount property.
+	// when VNET integration is enabled. It can be overridden per pool by setting the pool's IPAdddressCount property.
 	DefaultAgentMultiIPAddressCount = 128
 	// DefaultKubernetesClusterDomain is the dns suffix used in the cluster (used as a SAN in the PKI generation)
 	DefaultKubernetesClusterDomain = "cluster.local"
@@ -42,14 +42,15 @@ const (
 )
 
 const (
-	// Master represents the master node type
+	// DCOSMaster represents the master node type
 	DCOSMaster DCOSNodeType = "DCOSMaster"
-	// PrivateAgent represents the private agent node type
+	// DCOSPrivateAgent represents the private agent node type
 	DCOSPrivateAgent DCOSNodeType = "DCOSPrivateAgent"
-	// PublicAgent represents the public agent node type
+	// DCOSPublicAgent represents the public agent node type
 	DCOSPublicAgent DCOSNodeType = "DCOSPublicAgent"
 )
 
+// KubeImages represents Docker images used for Kubernetes components based on Kubernetes version
 var KubeImages = map[api.OrchestratorVersion]map[string]string{
 	api.Kubernetes166: {
 		"hyperkube":    "hyperkube-amd64:v1.6.6",

pkg/acsengine/defaults.go

@@ -21,11 +21,11 @@ var (
 		},
 
 		DCOSSpecConfig: DCOSSpecConfig{
-			DCOS173_BootstrapDownloadURL: fmt.Sprintf(MsecndDCOSBootstrapDownloadURL, "testing", "df308b6fc3bd91e1277baa5a3db928ae70964722"),
-			DCOS184_BootstrapDownloadURL: fmt.Sprintf(AzureEdgeDCOSBootstrapDownloadURL, "testing", "5b4aa43610c57ee1d60b4aa0751a1fb75824c083"),
-			DCOS187_BootstrapDownloadURL: fmt.Sprintf(AzureEdgeDCOSBootstrapDownloadURL, "stable", "e73ba2b1cd17795e4dcb3d6647d11a29b9c35084"),
-			DCOS188_BootstrapDownloadURL: fmt.Sprintf(AzureEdgeDCOSBootstrapDownloadURL, "stable", "5df43052907c021eeb5de145419a3da1898c58a5"),
-			DCOS190_BootstrapDownloadURL: fmt.Sprintf(AzureEdgeDCOSBootstrapDownloadURL, "stable", "58fd0833ce81b6244fc73bf65b5deb43217b0bd7"),
+			DCOS173BootstrapDownloadURL: fmt.Sprintf(MsecndDCOSBootstrapDownloadURL, "testing", "df308b6fc3bd91e1277baa5a3db928ae70964722"),
+			DCOS184BootstrapDownloadURL: fmt.Sprintf(AzureEdgeDCOSBootstrapDownloadURL, "testing", "5b4aa43610c57ee1d60b4aa0751a1fb75824c083"),
+			DCOS187BootstrapDownloadURL: fmt.Sprintf(AzureEdgeDCOSBootstrapDownloadURL, "stable", "e73ba2b1cd17795e4dcb3d6647d11a29b9c35084"),
+			DCOS188BootstrapDownloadURL: fmt.Sprintf(AzureEdgeDCOSBootstrapDownloadURL, "stable", "5df43052907c021eeb5de145419a3da1898c58a5"),
+			DCOS190BootstrapDownloadURL: fmt.Sprintf(AzureEdgeDCOSBootstrapDownloadURL, "stable", "58fd0833ce81b6244fc73bf65b5deb43217b0bd7"),
 		},
 	}
 
@@ -41,10 +41,10 @@ var (
 			KubeBinariesSASURLBase: "https://acs-mirror.azureedge.net/wink8s/",
 		},
 		DCOSSpecConfig: DCOSSpecConfig{
-			DCOS173_BootstrapDownloadURL: fmt.Sprintf(AzureChinaCloudDCOSBootstrapDownloadURL, "df308b6fc3bd91e1277baa5a3db928ae70964722"),
-			DCOS184_BootstrapDownloadURL: fmt.Sprintf(AzureChinaCloudDCOSBootstrapDownloadURL, "5b4aa43610c57ee1d60b4aa0751a1fb75824c083"),
-			DCOS187_BootstrapDownloadURL: fmt.Sprintf(AzureChinaCloudDCOSBootstrapDownloadURL, "e73ba2b1cd17795e4dcb3d6647d11a29b9c35084"),
-			DCOS188_BootstrapDownloadURL: fmt.Sprintf(AzureChinaCloudDCOSBootstrapDownloadURL, "5df43052907c021eeb5de145419a3da1898c58a5"),
+			DCOS173BootstrapDownloadURL: fmt.Sprintf(AzureChinaCloudDCOSBootstrapDownloadURL, "df308b6fc3bd91e1277baa5a3db928ae70964722"),
+			DCOS184BootstrapDownloadURL: fmt.Sprintf(AzureChinaCloudDCOSBootstrapDownloadURL, "5b4aa43610c57ee1d60b4aa0751a1fb75824c083"),
+			DCOS187BootstrapDownloadURL: fmt.Sprintf(AzureChinaCloudDCOSBootstrapDownloadURL, "e73ba2b1cd17795e4dcb3d6647d11a29b9c35084"),
+			DCOS188BootstrapDownloadURL: fmt.Sprintf(AzureChinaCloudDCOSBootstrapDownloadURL, "5df43052907c021eeb5de145419a3da1898c58a5"),
 		},
 	}
 )

pkg/acsengine/engine.go

@@ -169,10 +169,10 @@ type KeyVaultRef struct {
 	SecretVersion string     `json:"secretVersion,omitempty"`
 }
 
-var keyvaultSecretPath_re *regexp.Regexp
+var keyvaultSecretPathRe *regexp.Regexp
 
 func init() {
-	keyvaultSecretPath_re = regexp.MustCompile(`^(/subscriptions/\S+/resourceGroups/\S+/providers/Microsoft.KeyVault/vaults/\S+)/secrets/([^/\s]+)(/(\S+))?$`)
+	keyvaultSecretPathRe = regexp.MustCompile(`^(/subscriptions/\S+/resourceGroups/\S+/providers/Microsoft.KeyVault/vaults/\S+)/secrets/([^/\s]+)(/(\S+))?$`)
 }
 
 func (t *TemplateGenerator) verifyFiles() error {
@@ -402,20 +402,20 @@ func getParameters(cs *api.ContainerService, isClassicMode bool) (map[string]int
 	}
 
 	if strings.HasPrefix(string(properties.OrchestratorProfile.OrchestratorType), string(api.DCOS)) {
-		dcosBootstrapURL := cloudSpecConfig.DCOSSpecConfig.DCOS188_BootstrapDownloadURL
+		dcosBootstrapURL := cloudSpecConfig.DCOSSpecConfig.DCOS188BootstrapDownloadURL
 		switch properties.OrchestratorProfile.OrchestratorType {
 		case api.DCOS:
 			switch properties.OrchestratorProfile.OrchestratorVersion {
 			case api.DCOS173:
-				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS173_BootstrapDownloadURL
+				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS173BootstrapDownloadURL
 			case api.DCOS184:
-				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS184_BootstrapDownloadURL
+				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS184BootstrapDownloadURL
 			case api.DCOS187:
-				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS187_BootstrapDownloadURL
+				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS187BootstrapDownloadURL
 			case api.DCOS188:
-				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS188_BootstrapDownloadURL
+				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS188BootstrapDownloadURL
 			case api.DCOS190:
-				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS190_BootstrapDownloadURL
+				dcosBootstrapURL = cloudSpecConfig.DCOSSpecConfig.DCOS190BootstrapDownloadURL
 			}
 		}
 		addValue(parametersMap, "dcosBootstrapURL", dcosBootstrapURL)
@@ -468,7 +468,7 @@ func addSecret(m map[string]interface{}, k string, v interface{}, encode bool) {
 		addValue(m, k, v)
 		return
 	}
-	parts := keyvaultSecretPath_re.FindStringSubmatch(str)
+	parts := keyvaultSecretPathRe.FindStringSubmatch(str)
 	if parts == nil || len(parts) != 5 {
 		if encode {
 			addValue(m, k, base64.StdEncoding.EncodeToString([]byte(str)))
@@ -1283,12 +1283,12 @@ write_files:
 
 func getKubernetesSubnets(properties *api.Properties) string {
 	subnetString := `{
-            "name": "podCIDR%d", 
+            "name": "podCIDR%d",
             "properties": {
-              "addressPrefix": "10.244.%d.0/24", 
+              "addressPrefix": "10.244.%d.0/24",
               "networkSecurityGroup": {
                 "id": "[variables('nsgID')]"
-              }, 
+              },
               "routeTable": {
                 "id": "[variables('routeTableID')]"
               }

pkg/acsengine/pki.go

@@ -182,15 +182,15 @@ func privateKeyToPem(privateKey *rsa.PrivateKey) []byte {
 func pemToCertificate(raw string) (*x509.Certificate, error) {
 	cpb, _ := pem.Decode([]byte(raw))
 	if cpb == nil {
-		return nil, errors.New("The raw pem is not a valid PEM formatted block.")
+		return nil, errors.New("The raw pem is not a valid PEM formatted block")
 	}
 	return x509.ParseCertificate(cpb.Bytes)
 }
 
 func pemToKey(raw string) (*rsa.PrivateKey, error) {
 	kpb, _ := pem.Decode([]byte(raw))
 	if kpb == nil {
-		return nil, errors.New("The raw pem is not a valid PEM formatted block.")
+		return nil, errors.New("The raw pem is not a valid PEM formatted block")
 	}
 	return x509.ParsePKCS1PrivateKey(kpb.Bytes)
 }

pkg/acsengine/tenantid.go

@@ -10,7 +10,7 @@ import (
 	log "github.com/Sirupsen/logrus"
 )
 
-// findTenantID figures out the AAD tenant ID of the subscription by making an
+// GetTenantID figures out the AAD tenant ID of the subscription by making an
 // unauthenticated request to the Get Subscription Details endpoint and parses
 // the value from WWW-Authenticate header.
 func GetTenantID(env azure.Environment, subscriptionID string) (string, error) {

pkg/acsengine/types.go

@@ -32,11 +32,11 @@ type DockerSpecConfig struct {
 
 //DCOSSpecConfig is the configurations of DCOS
 type DCOSSpecConfig struct {
-	DCOS173_BootstrapDownloadURL string
-	DCOS184_BootstrapDownloadURL string
-	DCOS187_BootstrapDownloadURL string
-	DCOS188_BootstrapDownloadURL string
-	DCOS190_BootstrapDownloadURL string
+	DCOS173BootstrapDownloadURL string
+	DCOS184BootstrapDownloadURL string
+	DCOS187BootstrapDownloadURL string
+	DCOS188BootstrapDownloadURL string
+	DCOS190BootstrapDownloadURL string
 }
 
 //KubernetesSpecConfig is the kubernetes container images used.

pkg/api/convertertoupgradeapi.go

@@ -17,17 +17,3 @@ func ConvertVLabsUpgradeContainerService(vlabs *vlabs.UpgradeContainerService) *
 	convertVLabsOrchestratorProfile(vlabs.OrchestratorProfile, ucs.OrchestratorProfile)
 	return ucs
 }
-
-func convertVLabsUpgradeOrchestratorProfile(vlabscs *vlabs.OrchestratorProfile, api *OrchestratorProfile) {
-	api.OrchestratorType = OrchestratorType(vlabscs.OrchestratorType)
-	if api.OrchestratorType == Kubernetes {
-		switch vlabscs.OrchestratorVersion {
-		case vlabs.Kubernetes162:
-			api.OrchestratorVersion = Kubernetes162
-		case vlabs.Kubernetes160:
-			api.OrchestratorVersion = Kubernetes160
-		default:
-			api.OrchestratorVersion = KubernetesLatest
-		}
-	}
-}

pkg/api/v20160330/types.go

@@ -41,7 +41,7 @@ type Properties struct {
 	DiagnosticsProfile *DiagnosticsProfile `json:"diagnosticsProfile,omitempty"`
 
 	// JumpboxProfile has made it into the new ACS RP stack for
-	// backward compability.
+	// backward compatibility.
 	// TODO: Version this field so that newer versions don't
 	// allow jumpbox creation
 	JumpboxProfile *JumpboxProfile `json:"jumpboxProfile,omitempty"`

pkg/api/v20160330/validate.go

@@ -148,14 +148,3 @@ func validateUniqueProfileNames(profiles []*AgentPoolProfile) error {
 	}
 	return nil
 }
-
-func validateUniquePorts(ports []int, name string) error {
-	portMap := make(map[int]bool)
-	for _, port := range ports {
-		if _, ok := portMap[port]; ok {
-			return fmt.Errorf("agent profile '%s' has duplicate port '%d', ports must be unique", name, port)
-		}
-		portMap[port] = true
-	}
-	return nil
-}

pkg/api/v20160930/const.go

@@ -18,8 +18,10 @@ const (
 )
 
 const (
+	// Windows string constant for VMs
 	Windows OSType = "Windows"
-	Linux   OSType = "Linux"
+	// Linux string constant for VMs
+	Linux OSType = "Linux"
 )
 
 // validation values

pkg/api/v20160930/validate.go

@@ -164,14 +164,3 @@ func validateUniqueProfileNames(profiles []*AgentPoolProfile) error {
 	}
 	return nil
 }
-
-func validateUniquePorts(ports []int, name string) error {
-	portMap := make(map[int]bool)
-	for _, port := range ports {
-		if _, ok := portMap[port]; ok {
-			return fmt.Errorf("agent profile '%s' has duplicate port '%d', ports must be unique", name, port)
-		}
-		portMap[port] = true
-	}
-	return nil
-}

pkg/api/v20170131/const.go

@@ -20,8 +20,10 @@ const (
 )
 
 const (
+	// Windows string constant for VMs
 	Windows OSType = "Windows"
-	Linux   OSType = "Linux"
+	// Linux string constant for VMs
+	Linux OSType = "Linux"
 )
 
 // validation values

pkg/api/v20170131/validate.go

@@ -167,14 +167,3 @@ func validateUniqueProfileNames(profiles []*AgentPoolProfile) error {
 	}
 	return nil
 }
-
-func validateUniquePorts(ports []int, name string) error {
-	portMap := make(map[int]bool)
-	for _, port := range ports {
-		if _, ok := portMap[port]; ok {
-			return fmt.Errorf("agent profile '%s' has duplicate port '%d', ports must be unique", name, port)
-		}
-		portMap[port] = true
-	}
-	return nil
-}

pkg/api/vlabs/upgradetypesandvalidate.go

@@ -13,13 +13,13 @@ func (ucs *UpgradeContainerService) Validate() error {
 	case DCOS:
 	case Swarm:
 	case SwarmMode:
-		return fmt.Errorf("Upgrade is not supported for orchestrator: %s \n", ucs.OrchestratorProfile.OrchestratorType)
+		return fmt.Errorf("Upgrade is not supported for orchestrator: %s", ucs.OrchestratorProfile.OrchestratorType)
 	case Kubernetes:
 		switch ucs.OrchestratorProfile.OrchestratorVersion {
 		case Kubernetes162:
 		case Kubernetes160:
 		default:
-			return fmt.Errorf("Invalid orchestrator version: %s \n", ucs.OrchestratorProfile.OrchestratorVersion)
+			return fmt.Errorf("Invalid orchestrator version: %s", ucs.OrchestratorProfile.OrchestratorVersion)
 		}
 	}