Skip to content

Commit

Permalink
Fix version command to show correct information (#14754)
Browse files Browse the repository at this point in the history
* Fix version command to show correct information

A recent change made the ldflags not get passed properly leading to the
version command giving "unknown". This change fixes this issue, and adds
a tests to ensure similar changes are caught in the future.

During this it was discovered some of our binaries don't include the
version command, so those were added to sdsclient and mixgen.

* Fix lint and codecov

* Fix lint
  • Loading branch information
howardjohn authored and istio-testing committed Jun 14, 2019
1 parent a87f2e0 commit 0f775a4
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 2 deletions.
5 changes: 5 additions & 0 deletions Makefile
Expand Up @@ -370,6 +370,11 @@ ${ISTIO_OUT}/sdsclient:
# Build will rebuild the go binaries.
build: depend $(PILOT_GO_BINS_SHORT) mixc mixs mixgen node_agent node_agent_k8s istio_ca istioctl galley sdsclient

.PHONY: version-test
# Do not run istioctl since is different (connects to kubernetes)
version-test:
go test ./tests/version/... -v --base-dir ${ISTIO_OUT} --binaries="$(PILOT_GO_BINS_SHORT) mixc mixs mixgen node_agent node_agent_k8s istio_ca galley sdsclient"

# The following are convenience aliases for most of the go targets
# The first block is for aliases that are the same as the actual binary,
# while the ones that follow need slight adjustments to their names.
Expand Down
2 changes: 1 addition & 1 deletion bin/gobuild.sh
Expand Up @@ -36,7 +36,7 @@ GOBINARY=${GOBINARY:-go}
GOPKG="$GOPATH/pkg"
BUILDINFO=${BUILDINFO:-""}
STATIC=${STATIC:-1}
LDFLAGS=${LDFLAGS:-extldflags -static}
LDFLAGS=${LDFLAGS:--extldflags -static}
GOBUILDFLAGS=${GOBUILDFLAGS:-""}
# Split GOBUILDFLAGS by spaces into an array called GOBUILDFLAGS_ARRAY.
IFS=' ' read -r -a GOBUILDFLAGS_ARRAY <<< "$GOBUILDFLAGS"
Expand Down
1 change: 1 addition & 0 deletions codecov.skip
Expand Up @@ -18,6 +18,7 @@ istio.io/istio/samples
istio.io/istio/security/tests/integration
istio.io/istio/tests/codecov
istio.io/istio/tests/e2e
istio.io/istio/tests/version
istio.io/istio/tests/integration2/examples
istio.io/istio/tests/integration2/qualification
istio.io/istio/tests/integration_old
Expand Down
3 changes: 3 additions & 0 deletions mixer/tools/mixgen/main.go
Expand Up @@ -19,11 +19,14 @@ import (

"istio.io/istio/mixer/cmd/shared"
"istio.io/istio/mixer/tools/mixgen/cmd"
"istio.io/pkg/version"
)

func main() {
rootCmd := cmd.GetRootCmd(os.Args[1:], shared.Printf, shared.Fatalf)

rootCmd.AddCommand(version.CobraCommand())

if err := rootCmd.Execute(); err != nil {
os.Exit(-1)
}
Expand Down
2 changes: 1 addition & 1 deletion prow/istio-unit-tests.sh
Expand Up @@ -34,4 +34,4 @@ cd "${ROOT}"
# Integration/e2e tests in the other scripts are run against GKE or real clusters.
JUNIT_UNIT_TEST_XML="${ARTIFACTS_DIR}/junit_unit-tests.xml" \
T="-v" \
make build localTestEnv test
make build localTestEnv test version-test
6 changes: 6 additions & 0 deletions security/tools/sdsclient/main.go
Expand Up @@ -20,6 +20,8 @@ import (

"github.com/spf13/cobra"

"istio.io/pkg/version"

"istio.io/istio/pkg/cmd"
"istio.io/istio/security/pkg/testing/sdsc"
"istio.io/pkg/env"
Expand Down Expand Up @@ -48,6 +50,10 @@ var (
}
)

func init() {
rootCmd.AddCommand(version.CobraCommand())
}

func main() {
if err := rootCmd.Execute(); err != nil {
log.Fatalf("failed to start the sdsclient, error %v", err)
Expand Down
79 changes: 79 additions & 0 deletions tests/version/version_test.go
@@ -0,0 +1,79 @@
// Copyright 2019 Istio Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package version

import (
"encoding/json"
"flag"
"os/exec"
"path"
"strings"
"testing"

"istio.io/pkg/version"
)

var (
binaries *string
releasedir *string
)

func init() {
releasedir = flag.String("base-dir", "", "directory for binaries")
binaries = flag.String("binaries", "", "space separated binaries to test")
flag.Parse()

}

func TestVersion(t *testing.T) {
binariesToTest := strings.Split(*binaries, " ")
if len(binariesToTest) == 0 {
t.Fatal("No binaries to test. Pass the --binaries flag.")
}
for _, b := range binariesToTest {
cmd := path.Join(*releasedir, b)
t.Run(b, func(t *testing.T) {
out, err := exec.Command(cmd, "version", "-ojson").Output()
if err != nil {
t.Fatalf("Version failed with error: %v. Output: %v", err, string(out))
}

var resp version.Version
if err := json.Unmarshal(out, &resp); err != nil {
t.Fatalf("Failed to marshal to json: %v. Output: %v", err, string(out))
}

verInfo := resp.ClientVersion

validateField(t, "Version", verInfo.Version)
validateField(t, "GitRevision", verInfo.GitRevision)
validateField(t, "User", verInfo.User)
validateField(t, "Host", verInfo.Host)
validateField(t, "GolangVersion", verInfo.GolangVersion)
validateField(t, "DockerHub", verInfo.DockerHub)
validateField(t, "BuildStatus", verInfo.BuildStatus)
validateField(t, "GitTag", verInfo.GitTag)
})
}
}

// TODO we may be able to do more validation of fields here, but because it changes based on the environment this is not easy
// For now ensuring the fields even get populated is most important.
func validateField(t *testing.T, field, s string) {
t.Helper()
if s == "unknown" || s == "" {
t.Errorf("Field %v was invalid. Got: %v.", field, s)
}
}

0 comments on commit 0f775a4

Please sign in to comment.