Skip to content

Commit d12eff3

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #86 from DirectXMan12/bug/dont-use-std-log
Clean up logging usage (and minor script improvements)
2 parents 3aaf8cd + d3231aa commit d12eff3

37 files changed

+693
-331
lines changed

‎.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ install:
1818
- go get -u gopkg.in/alecthomas/gometalinter.v2 && gometalinter.v2 --install
1919

2020
script:
21-
- TRACE=1 ./test.sh
21+
- TRACE=1 ./hack/check-everything.sh
2222

2323

2424
# TBD. Suppressing for now.

‎example/main.go

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ package main
1919
import (
2020
"context"
2121
"flag"
22-
"log"
22+
"os"
2323

24+
"github.com/go-logr/logr"
2425
appsv1 "k8s.io/api/apps/v1"
2526
corev1 "k8s.io/api/core/v1"
2627
"k8s.io/apimachinery/pkg/api/errors"
@@ -36,63 +37,78 @@ import (
3637
"sigs.k8s.io/controller-runtime/pkg/source"
3738
)
3839

40+
var (
41+
log = logf.Log.WithName("example-controller")
42+
)
43+
3944
func main() {
4045
flag.Parse()
4146
logf.SetLogger(logf.ZapLogger(false))
47+
entryLog := log.WithName("entrypoint")
4248

4349
// Setup a Manager
44-
mrg, err := manager.New(config.GetConfigOrDie(), manager.Options{})
50+
mgr, err := manager.New(config.GetConfigOrDie(), manager.Options{})
4551
if err != nil {
46-
log.Fatal(err)
52+
entryLog.Error(err, "unable to set up overall controller manager")
53+
os.Exit(1)
4754
}
4855

4956
// Setup a new controller to Reconciler ReplicaSets
50-
c, err := controller.New("foo-controller", mrg, controller.Options{
51-
Reconciler: &reconcileReplicaSet{client: mrg.GetClient()},
57+
c, err := controller.New("foo-controller", mgr, controller.Options{
58+
Reconciler: &reconcileReplicaSet{client: mgr.GetClient(), log: log.WithName("reconciler")},
5259
})
5360
if err != nil {
54-
log.Fatal(err)
61+
entryLog.Error(err, "unable to set up individual controller")
62+
os.Exit(1)
5563
}
5664

5765
// Watch ReplicaSets and enqueue ReplicaSet object key
5866
if err := c.Watch(&source.Kind{Type: &appsv1.ReplicaSet{}}, &handler.EnqueueRequestForObject{}); err != nil {
59-
log.Fatal(err)
67+
entryLog.Error(err, "unable to watch ReplicaSets")
68+
os.Exit(1)
6069
}
6170

6271
// Watch Pods and enqueue owning ReplicaSet key
6372
if err := c.Watch(&source.Kind{Type: &corev1.Pod{}},
6473
&handler.EnqueueRequestForOwner{OwnerType: &appsv1.ReplicaSet{}, IsController: true}); err != nil {
65-
log.Fatal(err)
74+
entryLog.Error(err, "unable to watch Pods")
75+
os.Exit(1)
6676
}
6777

68-
log.Fatal(mrg.Start(signals.SetupSignalHandler()))
78+
if err := mgr.Start(signals.SetupSignalHandler()); err != nil {
79+
entryLog.Error(err, "unable to run manager")
80+
os.Exit(1)
81+
}
6982
}
7083

7184
// reconcileReplicaSet reconciles ReplicaSets
7285
type reconcileReplicaSet struct {
7386
client client.Client
87+
log logr.Logger
7488
}
7589

7690
// Implement reconcile.Reconciler so the controller can reconcile objects
7791
var _ reconcile.Reconciler = &reconcileReplicaSet{}
7892

7993
func (r *reconcileReplicaSet) Reconcile(request reconcile.Request) (reconcile.Result, error) {
94+
// set up a convinient log object so we don't have to type request over and over again
95+
log := r.log.WithValues("request", request)
96+
8097
// Fetch the ReplicaSet from the cache
8198
rs := &appsv1.ReplicaSet{}
8299
err := r.client.Get(context.TODO(), request.NamespacedName, rs)
83100
if errors.IsNotFound(err) {
84-
log.Printf("Could not find ReplicaSet %v.\n", request)
101+
log.Error(nil, "Could not find ReplicaSet")
85102
return reconcile.Result{}, nil
86103
}
87104

88105
if err != nil {
89-
log.Printf("Could not fetch ReplicaSet %v for %+v\n", err, request)
106+
log.Error(err, "Could not fetch ReplicaSet")
90107
return reconcile.Result{}, err
91108
}
92109

93110
// Print the ReplicaSet
94-
log.Printf("ReplicaSet Name %s Namespace %s, Pod Name: %s\n",
95-
rs.Name, rs.Namespace, rs.Spec.Template.Spec.Containers[0].Name)
111+
log.Info("Reconciling ReplicaSet", "container name", rs.Spec.Template.Spec.Containers[0].Name)
96112

97113
// Set the label if it is missing
98114
if rs.Labels == nil {
@@ -106,7 +122,7 @@ func (r *reconcileReplicaSet) Reconcile(request reconcile.Request) (reconcile.Re
106122
rs.Labels["hello"] = "world"
107123
err = r.client.Update(context.TODO(), rs)
108124
if err != nil {
109-
log.Printf("Could not write ReplicaSet %v\n", err)
125+
log.Error(err, "Could not write ReplicaSet")
110126
return reconcile.Result{}, err
111127
}
112128

‎hack/check-everything.sh

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
#!/usr/bin/env bash
2+
3+
# Copyright 2018 The Kubernetes Authors.
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
set -e
18+
19+
hack_dir=$(dirname ${BASH_SOURCE})
20+
source ${hack_dir}/common.sh
21+
22+
k8s_version=1.10.1
23+
goarch=amd64
24+
goos="unknown"
25+
26+
if [[ "$OSTYPE" == "linux-gnu" ]]; then
27+
goos="linux"
28+
elif [[ "$OSTYPE" == "darwin"* ]]; then
29+
goos="darwin"
30+
fi
31+
32+
if [[ "$goos" == "unknown" ]]; then
33+
echo "OS '$OSTYPE' not supported. Aborting." >&2
34+
exit 1
35+
fi
36+
37+
tmp_root=/tmp
38+
kb_root_dir=$tmp_root/kubebuilder
39+
40+
# Skip fetching and untaring the tools by setting the SKIP_FETCH_TOOLS variable
41+
# in your environment to any value:
42+
#
43+
# $ SKIP_FETCH_TOOLS=1 ./test.sh
44+
#
45+
# If you skip fetching tools, this script will use the tools already on your
46+
# machine, but rebuild the kubebuilder and kubebuilder-bin binaries.
47+
SKIP_FETCH_TOOLS=${SKIP_FETCH_TOOLS:-""}
48+
49+
# fetch k8s API gen tools and make it available under kb_root_dir/bin.
50+
function fetch_kb_tools {
51+
if [ -n "$SKIP_FETCH_TOOLS" ]; then
52+
return 0
53+
fi
54+
55+
header_text "fetching tools"
56+
kb_tools_archive_name="kubebuilder-tools-$k8s_version-$goos-$goarch.tar.gz"
57+
kb_tools_download_url="https://storage.googleapis.com/kubebuilder-tools/$kb_tools_archive_name"
58+
59+
kb_tools_archive_path="$tmp_root/$kb_tools_archive_name"
60+
if [ ! -f $kb_tools_archive_path ]; then
61+
curl -sL ${kb_tools_download_url} -o "$kb_tools_archive_path"
62+
fi
63+
tar -zvxf "$kb_tools_archive_path" -C "$tmp_root/"
64+
}
65+
66+
header_text "using tools"
67+
68+
which gometalinter.v2
69+
fetch_kb_tools
70+
setup_envs
71+
72+
${hack_dir}/verify.sh
73+
${hack_dir}/test-all.sh
74+
75+
header_text "confirming example compiles (via go install)"
76+
go install ./example
77+
78+
echo "passed"
79+
exit 0

‎hack/common.sh

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
#!/usr/bin/env bash
2+
3+
# Copyright 2018 The Kubernetes Authors.
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
set -e
18+
19+
# Enable tracing in this script off by setting the TRACE variable in your
20+
# environment to any value:
21+
#
22+
# $ TRACE=1 test.sh
23+
TRACE=${TRACE:-""}
24+
if [ -n "$TRACE" ]; then
25+
set -x
26+
fi
27+
28+
# Turn colors in this script off by setting the NO_COLOR variable in your
29+
# environment to any value:
30+
#
31+
# $ NO_COLOR=1 test.sh
32+
NO_COLOR=${NO_COLOR:-""}
33+
if [ -z "$NO_COLOR" ]; then
34+
header=$'\e[1;33m'
35+
reset=$'\e[0m'
36+
else
37+
header=''
38+
reset=''
39+
fi
40+
41+
function header_text {
42+
echo "$header$*$reset"
43+
}
44+
45+
function setup_envs {
46+
header_text "setting up env vars"
47+
48+
# Setup env vars
49+
if [[ -z "${KUBEBUILDER_ASSETS}" ]]; then
50+
export KUBEBUILDER_ASSETS=$kb_root_dir/bin
51+
fi
52+
}

‎hack/test-all.sh

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#!/usr/bin/env bash
2+
3+
# Copyright 2018 The Kubernetes Authors.
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
set -e
18+
19+
source $(dirname ${BASH_SOURCE})/common.sh
20+
21+
setup_envs
22+
23+
header_text "running go test"
24+
25+
go test ./pkg/... -parallel 4
26+
27+
header_text "running coverage"
28+
29+
# Verify no coverage regressions have been introduced. Remove the exception list from here
30+
# once the coverage has been brought back up
31+
if [[ ! $(go test ./pkg/... -coverprofile cover.out -parallel 4 | grep -v "coverage: 100.0% of statements" | grep "controller-runtime/pkg " | grep -v "controller-runtime/pkg \|controller-runtime/pkg/recorder \|pkg/admission/certprovisioner \|pkg/internal/admission \|pkg/cache\|pkg/client \|pkg/event \|pkg/client/config \|pkg/controller/controllertest \|pkg/reconcile/reconciletest \|pkg/test ") ]]; then
32+
echo "ok"
33+
else
34+
go test ./pkg/... -coverprofile cover.out -parallel 4 | grep -v "coverage: 100.0% of statements" | grep "controller-runtime/pkg " | grep -v "controller-runtime/pkg \|controller-runtime/pkg/recorder \|pkg/admission/certprovisioner \|pkg/internal/admission \|pkg/cache\|pkg/client \|pkg/event \|pkg/client/config \|pkg/controller/controllertest \|pkg/reconcile/reconciletest \|pkg/test "
35+
echo "missing test coverage"
36+
exit 1
37+
fi

‎hack/verify.sh

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
#!/usr/bin/env bash
2+
3+
# Copyright 2018 The Kubernetes Authors.
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
set -e
18+
19+
source $(dirname ${BASH_SOURCE})/common.sh
20+
21+
header_text "running go vet"
22+
23+
go vet ./pkg/...
24+
25+
# go get is broken for golint. re-enable this once it is fixed.
26+
#header_text "running golint"
27+
#
28+
#golint -set_exit_status ./pkg/...
29+
30+
header_text "running gometalinter.v2"
31+
32+
gometalinter.v2 --disable-all \
33+
--deadline 5m \
34+
--enable=misspell \
35+
--enable=structcheck \
36+
--enable=golint \
37+
--enable=deadcode \
38+
--enable=goimports \
39+
--enable=errcheck \
40+
--enable=varcheck \
41+
--enable=goconst \
42+
--enable=gas \
43+
--enable=unparam \
44+
--enable=ineffassign \
45+
--enable=nakedret \
46+
--enable=interfacer \
47+
--enable=misspell \
48+
--enable=gocyclo \
49+
--line-length=170 \
50+
--enable=lll \
51+
--dupl-threshold=400 \
52+
--enable=dupl \
53+
--skip=atomic \
54+
./pkg/...
55+
# TODO: Enable these as we fix them to make them pass
56+
# --enable=maligned \
57+
# --enable=safesql \

‎pkg/admission/cert/writer/certwriter.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"encoding/pem"
2424
"errors"
2525
"fmt"
26-
"log"
2726
"net/url"
2827
"time"
2928

@@ -101,7 +100,7 @@ func handleCommon(webhook *admissionregistrationv1beta1.Webhook, ch certReadWrit
101100
// Recreate the cert if it's invalid.
102101
valid := validCert(certs, dnsName)
103102
if !valid {
104-
log.Printf("cert is invalid or expiring, regenerating a new one")
103+
log.Info("cert is invalid or expiring, regenerating a new one")
105104
certs, err = ch.overwrite(webhook.Name)
106105
if err != nil {
107106
return err

‎pkg/admission/cert/writer/doc.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,9 @@ the desired path.
8181
8282
*/
8383
package writer
84+
85+
import (
86+
logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
87+
)
88+
89+
var log = logf.KBLog.WithName("admission").WithName("cert").WithName("writer")

0 commit comments

Comments
 (0)