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

Change go version detection logic #10101

Merged
merged 2 commits into from
Aug 11, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ install:
- make install-travis

script:
- make verify
- PERMISSIVE_GO=y make verify

notifications:
irc: "chat.freenode.net#openshift-dev"
37 changes: 37 additions & 0 deletions hack/lib/util/golang.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#!/bin/bash
#
# This library holds golang related utility functions.

# os::golang::verify_go_version ensure the go tool exists and is a viable version.
function os::golang::verify_go_version() {
if ! which go &>/dev/null; then
os::log::error "Can't find 'go' in PATH, please fix and retry."
os::log::error "See http://golang.org/doc/install for installation instructions."
return 1
fi

local go_version
go_version=($(go version))
if [[ "${go_version[2]}" != go1.6* ]]; then
os::log::info "Detected go version: ${go_version[*]}."
if [[ -z "${PERMISSIVE_GO:-}" ]]; then
os::log::error "Please install Go version 1.6 or use PERMISSIVE_GO=y to bypass this check."
return 1
else
os::log::warn "Detected golang version doesn't match preferred Go version for Origin."
os::log::warn "This version mismatch could lead to differences in execution between this run and the Origin CI systems."
return 0
fi
fi
}
readonly -f os::golang::verify_go_version

# os::golang::verify_golint_version ensure the golint tool exists.
function os::golang::verify_golint_version() {
if ! which golint &>/dev/null; then
os::log::error "Unable to detect 'golint' package."
os::log::error "To install it, run: 'go get github.com/golang/lint/golint'."
return 1
fi
}
readonly -f os::golang::verify_golint_version
19 changes: 7 additions & 12 deletions hack/verify-gofmt.sh
Original file line number Diff line number Diff line change
@@ -6,23 +6,18 @@ set -o errexit
set -o nounset
set -o pipefail

GO_VERSION=($(go version))

if [[ -z $(echo "${GO_VERSION[2]}" | grep -E 'go1.6') && -z "${FORCE_VERIFY-}" ]]; then
echo "Unknown go version '${GO_VERSION}', skipping gofmt." >&2
exit 0
fi

OS_ROOT=$(dirname "${BASH_SOURCE}")/..
source "${OS_ROOT}/hack/lib/init.sh"

os::golang::verify_go_version

cd "${OS_ROOT}"

bad_files=$(find_files | xargs gofmt -s -l)
if [[ -n "${bad_files}" ]]; then
echo "!!! gofmt needs to be run on the following files: " >&2
echo "${bad_files}"
echo "Try running 'gofmt -s -d [path]'" >&2
echo "Or autocorrect with 'hack/verify-gofmt.sh | xargs -n 1 gofmt -s -w'" >&2
exit 1
echo "!!! gofmt needs to be run on the following files: " >&2
echo "${bad_files}"
echo "Try running 'gofmt -s -d [path]'" >&2
echo "Or autocorrect with 'hack/verify-gofmt.sh | xargs -n 1 gofmt -s -w'" >&2
exit 1
fi
54 changes: 22 additions & 32 deletions hack/verify-golint.sh
Original file line number Diff line number Diff line change
@@ -3,49 +3,39 @@
set -o errexit
set -o pipefail

if ! which golint &>/dev/null; then
echo "Unable to detect 'golint' package"
echo "To install it, run: 'go get github.com/golang/lint/golint'"
exit 1
fi

GO_VERSION=($(go version))

if [[ -z $(echo "${GO_VERSION[2]}" | grep -E 'go1.6') && -z "${FORCE_VERIFY-}" ]]; then
echo "Unknown go version '${GO_VERSION}', skipping golint."
exit 0
fi

OS_ROOT=$(dirname "${BASH_SOURCE}")/..
source "${OS_ROOT}/hack/lib/init.sh"

os::golang::verify_go_version
os::golang::verify_golint_version

cd "${OS_ROOT}"

arg="${1:-""}"
bad_files=""

if [ "$arg" == "-m" ]; then
head=$(git rev-parse --short HEAD | xargs echo -n)
set +e
modified_files=$(git diff-tree --no-commit-id --name-only -r master..$head | \
grep "^pkg" | grep ".go$" | grep -v "bindata.go$" | grep -v "Godeps" | \
grep -v "third_party")
if [ -n "${modified_files}" ]; then
echo -e "Checking modified files: ${modified_files}\n"
for f in $modified_files; do golint $f; done
echo
fi
set -e
head=$(git rev-parse --short HEAD | xargs echo -n)
set +e
modified_files=$(git diff-tree --no-commit-id --name-only -r master..$head | \
grep "^pkg" | grep ".go$" | grep -v "bindata.go$" | grep -v "Godeps" | \
grep -v "third_party")
if [ -n "${modified_files}" ]; then
echo -e "Checking modified files: ${modified_files}\n"
for f in $modified_files; do golint $f; done
echo
fi
set -e
else
bad_files=$(find_files |
sort -u |
sed 's/^.{2}//' |
xargs -n1 printf "${GOPATH}/src/${OS_GO_PACKAGE}/%s\n" |
xargs -n1 golint)
bad_files=$(find_files | \
sort -u | \
sed 's/^.{2}//' | \
xargs -n1 printf "${GOPATH}/src/${OS_GO_PACKAGE}/%s\n" | \
xargs -n1 golint)
fi

if [[ -n "${bad_files}" ]]; then
echo "golint detected following problems:"
echo "${bad_files}"
exit 1
echo "golint detected following problems:"
echo "${bad_files}"
exit 1
fi
56 changes: 34 additions & 22 deletions hack/verify-govet.sh
Original file line number Diff line number Diff line change
@@ -3,30 +3,43 @@
set -o nounset
set -o pipefail

GO_VERSION=($(go version))

if [[ -z $(echo "${GO_VERSION[2]}" | grep -E 'go1.[6]') && -z "${FORCE_VERIFY-}" ]]; then
echo "Unknown go version '${GO_VERSION}', skipping go vet."
exit 0
fi

OS_ROOT=$(dirname "${BASH_SOURCE}")/..
source "${OS_ROOT}/hack/lib/init.sh"

os::golang::verify_go_version

cd "${OS_ROOT}"
mkdir -p _output/govet

os::build::setup_env

FAILURE=false
test_dirs=$(find_files | cut -d '/' -f 1-2 | sort -u)
for test_dir in $test_dirs
do
go tool vet -shadow=false $test_dir
if [ "$?" -ne 0 ]
then
FAILURE=true
fi
govet_blacklist=(
"pkg/auth/ldaputil/client.go:[0-9]+: assignment copies lock value to c: crypto/tls.Config contains sync.Once contains sync.Mutex"
"pkg/.*/client/clientset_generated/internalclientset/fake/clientset_generated.go:[0-9]+: literal copies lock value from fakePtr: github.com/openshift/origin/vendor/k8s.io/kubernetes/pkg/client/testing/core.Fake"
"pkg/.*/client/clientset_generated/release_1_3/fake/clientset_generated.go:30: literal copies lock value from fakePtr: github.com/openshift/origin/vendor/k8s.io/kubernetes/pkg/client/testing/core.Fake"
)

function govet_blacklist_contains() {
local text=$1
for blacklist_entry in "${govet_blacklist[@]}"; do
if grep -Eqx "${blacklist_entry}" <<<"${text}"; then
# the text we got matches this blacklist entry
return 0
fi
done
return 1
}

test_dirs="$(find_files | cut -d '/' -f 1-2 | sort -u)"
for test_dir in ${test_dirs}; do
if ! result="$(go tool vet -shadow=false "${test_dir}" 2>&1)"; then
while read -r line; do
if ! govet_blacklist_contains "${line}"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

@soltysh we don't want to remove this part of hack/verify-govet.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why, I haven't see us using it?

echo "${line}"
FAILURE=true
fi
done <<<"${result}"
fi
done

# For the sake of slowly white-listing `shadow` checks, we need to keep track of which
@@ -74,11 +87,10 @@ done

# We don't want to exit on the first failure of go vet, so just keep track of
# whether a failure occurred or not.
if $FAILURE
then
echo "FAILURE: go vet failed!"
exit 1
if [[ -n "${FAILURE:-}" ]]; then
echo "FAILURE: go vet failed!"
exit 1
else
echo "SUCCESS: go vet succeded!"
exit 0
echo "SUCCESS: go vet succeded!"
exit 0
fi
4 changes: 2 additions & 2 deletions pkg/api/graph/graph.go
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ type Node struct {

// DOTAttributes implements an attribute getter for the DOT encoding
func (n Node) DOTAttributes() []dot.Attribute {
return []dot.Attribute{{"label", fmt.Sprintf("%q", n.UniqueName)}}
return []dot.Attribute{{Key: "label", Value: fmt.Sprintf("%q", n.UniqueName)}}
}

// ExistenceChecker is an interface for those nodes that can be created without a backing object.
@@ -96,7 +96,7 @@ func (e Edge) IsKind(kind string) bool {

// DOTAttributes implements an attribute getter for the DOT encoding
func (e Edge) DOTAttributes() []dot.Attribute {
return []dot.Attribute{{"label", fmt.Sprintf("%q", strings.Join(e.Kinds().List(), ","))}}
return []dot.Attribute{{Key: "label", Value: fmt.Sprintf("%q", strings.Join(e.Kinds().List(), ","))}}
}

type GraphDescriber interface {
2 changes: 1 addition & 1 deletion pkg/build/admission/buildpodutil_test.go
Original file line number Diff line number Diff line change
@@ -95,7 +95,7 @@ func TestSetBuildLogLevel(t *testing.T) {

build = u.Build().WithSourceStrategy()
pod = u.Pod().WithEnvVar("BUILD", "foo")
build.Spec.Strategy.SourceStrategy.Env = []kapi.EnvVar{{"BUILD_LOGLEVEL", "7", nil}}
build.Spec.Strategy.SourceStrategy.Env = []kapi.EnvVar{{Name: "BUILD_LOGLEVEL", Value: "7", ValueFrom: nil}}
SetBuildLogLevel(pod.ToAttributes(), build.AsBuild())

if pod.Spec.Containers[0].Args[0] != "--loglevel=7" {
8 changes: 5 additions & 3 deletions pkg/cmd/util/net.go
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ import (
"strings"
"time"

knet "k8s.io/kubernetes/pkg/util/net"
"k8s.io/kubernetes/pkg/util/sets"

"github.com/golang/glog"
@@ -123,8 +124,9 @@ func TransportFor(ca string, certFile string, keyFile string) (http.RoundTripper
}

// Copy default transport
transport := *http.DefaultTransport.(*http.Transport)
transport.TLSClientConfig = &tls.Config{}
transport := knet.SetTransportDefaults(&http.Transport{
Copy link
Contributor

Choose a reason for hiding this comment

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

@soltysh what did we gain from using the k8s lib?

Copy link
Contributor

Choose a reason for hiding this comment

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

the intent of this was to pick up default transport settings, which the lib does safely

TLSClientConfig: &tls.Config{},
})

if len(ca) != 0 {
roots, err := CertPoolFromFile(ca)
@@ -142,7 +144,7 @@ func TransportFor(ca string, certFile string, keyFile string) (http.RoundTripper
transport.TLSClientConfig.Certificates = []tls.Certificate{cert}
}

return &transport, nil
return transport, nil
}

// GetCertificateFunc returns a function that can be used in tls.Config#GetCertificate
12 changes: 6 additions & 6 deletions pkg/route/generator/generate.go
Original file line number Diff line number Diff line change
@@ -21,12 +21,12 @@ var _ kubectl.Generator = RouteGenerator{}
// ParamNames returns the parameters required for generating a route
func (RouteGenerator) ParamNames() []kubectl.GeneratorParam {
return []kubectl.GeneratorParam{
{"labels", false},
{"default-name", true},
{"port", false},
{"name", false},
{"hostname", false},
{"path", false},
{Name: "labels", Required: false},
{Name: "default-name", Required: true},
{Name: "port", Required: false},
{Name: "name", Required: false},
{Name: "hostname", Required: false},
{Name: "path", Required: false},
}
}

4 changes: 2 additions & 2 deletions test/integration/dns_test.go
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@ func TestDNS(t *testing.T) {
waitutil.Until(func() {
m1 := &dns.Msg{
MsgHdr: dns.MsgHdr{Id: dns.Id(), RecursionDesired: false},
Question: []dns.Question{{"kubernetes.default.svc.cluster.local.", dns.TypeA, dns.ClassINET}},
Question: []dns.Question{{Name: "kubernetes.default.svc.cluster.local.", Qtype: dns.TypeA, Qclass: dns.ClassINET}},
}
in, err := dns.Exchange(m1, masterConfig.DNSConfig.BindAddress)
if err != nil {
@@ -241,7 +241,7 @@ func TestDNS(t *testing.T) {
}
m1 := &dns.Msg{
MsgHdr: dns.MsgHdr{Id: dns.Id(), RecursionDesired: tc.recursionExpected},
Question: []dns.Question{{tc.dnsQuestionName, qType, dns.ClassINET}},
Question: []dns.Question{{Name: tc.dnsQuestionName, Qtype: qType, Qclass: dns.ClassINET}},
}
ch := make(chan struct{})
count := 0