Skip to content

Commit

Permalink
UPSTREAM: <DEBUG>: Revert "Merge pull request kubernetes#112120 from …
Browse files Browse the repository at this point in the history
…pohly/klog-flag-removal"

This reverts commit 6dbec8e, reversing
changes made to 17c5066.
  • Loading branch information
sanchezl committed Dec 20, 2022
1 parent c6fedc2 commit e250f13
Show file tree
Hide file tree
Showing 16 changed files with 77 additions and 72 deletions.
3 changes: 1 addition & 2 deletions hack/jenkins/benchmark-dockerized.sh
Expand Up @@ -66,8 +66,7 @@ cd "${GOPATH}/src/k8s.io/kubernetes"
./hack/install-etcd.sh

# Run the benchmark tests and pretty-print the results into a separate file.
# Log output of the tests go to stderr.
make test-integration WHAT="$*" KUBE_TEST_ARGS="-run='XXX' -bench=${TEST_PREFIX:-.} -benchtime=${BENCHTIME:-1s} -benchmem -data-items-dir=${ARTIFACTS}" \
make test-integration WHAT="$*" KUBE_TEST_ARGS="-run='XXX' -bench=${TEST_PREFIX:-.} -benchtime=${BENCHTIME:-1s} -benchmem -alsologtostderr=false -logtostderr=false -data-items-dir=${ARTIFACTS}" \
| (go run test/integration/benchmark/extractlog/main.go) \
| tee \
>(prettybench -no-passthrough > "${ARTIFACTS}/BenchmarkResults.txt") \
Expand Down
6 changes: 3 additions & 3 deletions hack/make-rules/test-e2e-node.sh
Expand Up @@ -162,7 +162,7 @@ if [ "${remote}" = true ] && [ "${remote_mode}" = gce ] ; then
echo "Kubelet Config File: ${kubelet_config_file}"

# Invoke the runner
go run test/e2e_node/runner/remote/run_remote.go --vmodule=*=4 --ssh-env="gce" \
go run test/e2e_node/runner/remote/run_remote.go --logtostderr --vmodule=*=4 --ssh-env="gce" \
--zone="${zone}" --project="${project}" --gubernator="${gubernator}" \
--hosts="${hosts}" --images="${images}" --cleanup="${cleanup}" \
--results-dir="${artifacts}" --ginkgo-flags="${ginkgoflags}" --runtime-config="${runtime_config}" \
Expand All @@ -189,7 +189,7 @@ elif [ "${remote}" = true ] && [ "${remote_mode}" = ssh ] ; then
test_args='--kubelet-flags="--cluster-domain='${KUBE_DNS_DOMAIN:-cluster.local}'" '${test_args}

# Invoke the runner
go run test/e2e_node/runner/remote/run_remote.go --mode="ssh" --vmodule=*=4 \
go run test/e2e_node/runner/remote/run_remote.go --mode="ssh" --logtostderr --vmodule=*=4 \
--hosts="${hosts}" --results-dir="${artifacts}" --ginkgo-flags="${ginkgoflags}" \
--test_args="${test_args}" --system-spec-name="${system_spec_name}" \
--runtime-config="${runtime_config}" \
Expand Down Expand Up @@ -222,7 +222,7 @@ else
go run test/e2e_node/runner/local/run_local.go \
--system-spec-name="${system_spec_name}" --extra-envs="${extra_envs}" \
--ginkgo-flags="${ginkgoflags}" \
--test-flags="--v 4 --report-dir=${artifacts} --node-name $(hostname) ${test_args}" \
--test-flags="--alsologtostderr --v 4 --report-dir=${artifacts} --node-name $(hostname) ${test_args}" \
--runtime-config="${runtime_config}" \
--kubelet-config-file="${kubelet_config_file}" \
--build-dependencies=true 2>&1 | tee -i "${artifacts}/build-log.txt"
Expand Down
2 changes: 1 addition & 1 deletion hack/make-rules/test-integration.sh
Expand Up @@ -78,7 +78,7 @@ runTests() {
make -C "${KUBE_ROOT}" test \
WHAT="${WHAT:-$(kube::test::find_integration_test_dirs | paste -sd' ' -)}" \
GOFLAGS="${GOFLAGS:-}" \
KUBE_TEST_ARGS="${SHORT:--short=true} --vmodule=${KUBE_TEST_VMODULE} ${KUBE_TEST_ARGS:-}" \
KUBE_TEST_ARGS="--alsologtostderr=true ${SHORT:--short=true} --vmodule=${KUBE_TEST_VMODULE} ${KUBE_TEST_ARGS:-}" \
KUBE_TIMEOUT="${KUBE_TIMEOUT}" \
KUBE_RACE=""

Expand Down
1 change: 1 addition & 0 deletions hack/update-openapi-spec.sh
Expand Up @@ -84,6 +84,7 @@ kube::log::status "Starting kube-apiserver"
--service-account-lookup="${SERVICE_ACCOUNT_LOOKUP}" \
--service-account-issuer="https://kubernetes.default.svc" \
--service-account-signing-key-file="${SERVICE_ACCOUNT_KEY}" \
--logtostderr \
--v=2 \
--service-cluster-ip-range="10.0.0.0/24" >"${API_LOGFILE}" 2>&1 &
APISERVER_PID=$!
Expand Down
Expand Up @@ -62,7 +62,7 @@ func TestAddGlobalFlags(t *testing.T) {
}{
{
// Happy case
expectedFlag: []string{"help", "log-flush-frequency", "v", "vmodule"},
expectedFlag: []string{"add-dir-header", "alsologtostderr", "help", "log-backtrace-at", "log-dir", "log-file", "log-file-max-size", "log-flush-frequency", "logtostderr", "one-output", "skip-headers", "skip-log-headers", "stderrthreshold", "v", "vmodule"},
matchExpected: false,
},
{
Expand Down
23 changes: 20 additions & 3 deletions staging/src/k8s.io/component-base/logs/api/v1/options.go
Expand Up @@ -20,6 +20,7 @@ import (
"flag"
"fmt"
"math"
"sort"
"strings"
"time"

Expand All @@ -31,7 +32,6 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
cliflag "k8s.io/component-base/cli/flag"
"k8s.io/component-base/featuregate"
"k8s.io/component-base/logs/klogflags"
)

const (
Expand Down Expand Up @@ -183,8 +183,12 @@ func apply(c *LoggingConfiguration, featureGate featuregate.FeatureGate) error {

// AddFlags adds command line flags for the configuration.
func AddFlags(c *LoggingConfiguration, fs *pflag.FlagSet) {
// The help text is generated assuming that flags will eventually use
// hyphens, even if currently no normalization function is set for the
// flag set yet.
unsupportedFlags := strings.Join(unsupportedLoggingFlagNames(cliflag.WordSepNormalizeFunc), ", ")
formats := logRegistry.list()
fs.StringVar(&c.Format, "logging-format", c.Format, fmt.Sprintf("Sets the log format. Permitted formats: %s.", formats))
fs.StringVar(&c.Format, "logging-format", c.Format, fmt.Sprintf("Sets the log format. Permitted formats: %s.\nNon-default formats don't honor these flags: %s.\nNon-default choices are currently alpha and subject to change without warning.", formats, unsupportedFlags))
// No new log formats should be added after generation is of flag options
logRegistry.freeze()

Expand Down Expand Up @@ -232,13 +236,14 @@ var loggingFlags pflag.FlagSet

func init() {
var fs flag.FlagSet
klogflags.Init(&fs)
klog.InitFlags(&fs)
loggingFlags.AddGoFlagSet(&fs)
}

// List of logs (k8s.io/klog + k8s.io/component-base/logs) flags supported by all logging formats
var supportedLogsFlags = map[string]struct{}{
"v": {},
// TODO: support vmodule after 1.19 Alpha
}

// unsupportedLoggingFlags lists unsupported logging flags. The normalize
Expand All @@ -263,3 +268,15 @@ func unsupportedLoggingFlags(normalizeFunc func(f *pflag.FlagSet, name string) p
})
return allFlags
}

// unsupportedLoggingFlagNames lists unsupported logging flags by name, with
// optional normalization and sorted.
func unsupportedLoggingFlagNames(normalizeFunc func(f *pflag.FlagSet, name string) pflag.NormalizedName) []string {
unsupportedFlags := unsupportedLoggingFlags(normalizeFunc)
names := make([]string, 0, len(unsupportedFlags))
for _, f := range unsupportedFlags {
names = append(names, "--"+f.Name)
}
sort.Strings(names)
return names
}
Expand Up @@ -39,7 +39,9 @@ func TestFlags(t *testing.T) {
fs.SetOutput(&output)
fs.PrintDefaults()
want := ` --log-flush-frequency duration Maximum number of seconds between log flushes (default 5s)
--logging-format string Sets the log format. Permitted formats: "text". (default "text")
--logging-format string Sets the log format. Permitted formats: "text".
Non-default formats don't honor these flags: --add-dir-header, --alsologtostderr, --log-backtrace-at, --log-dir, --log-file, --log-file-max-size, --logtostderr, --one-output, --skip-headers, --skip-log-headers, --stderrthreshold, --vmodule.
Non-default choices are currently alpha and subject to change without warning. (default "text")
-v, --v Level number for the log level verbosity
--vmodule pattern=N,... comma-separated list of pattern=N settings for file-filtered logging (only works for text log format)
`
Expand Down
41 changes: 0 additions & 41 deletions staging/src/k8s.io/component-base/logs/klogflags/klogflags.go

This file was deleted.

48 changes: 37 additions & 11 deletions staging/src/k8s.io/component-base/logs/logs.go
Expand Up @@ -27,11 +27,16 @@ import (

"github.com/spf13/pflag"
logsapi "k8s.io/component-base/logs/api/v1"
"k8s.io/component-base/logs/klogflags"
"k8s.io/klog/v2"
)

const vmoduleUsage = " (only works for the default text log format)"
const deprecated = "will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components"

// TODO (https://github.com/kubernetes/kubernetes/issues/105310): once klog
// flags are removed, stop warning about "Non-default formats don't honor these
// flags" in config.go and instead add this remark here.
//
// const vmoduleUsage = " (only works for the default text log format)"

var (
packageFlags = flag.NewFlagSet("logging", flag.ContinueOnError)
Expand All @@ -42,7 +47,7 @@ var (
)

func init() {
klogflags.Init(packageFlags)
klog.InitFlags(packageFlags)
packageFlags.DurationVar(&logFlushFreq, logsapi.LogFlushFreqFlagName, logsapi.LogFlushFreqDefault, "Maximum number of seconds between log flushes")
}

Expand Down Expand Up @@ -76,29 +81,42 @@ var NewOptions = logsapi.NewLoggingConfiguration
//
// May be called more than once.
func AddFlags(fs *pflag.FlagSet, opts ...Option) {
// Determine whether the flags are already present by looking up one
// which always should exist.
if fs.Lookup("logtostderr") != nil {
return
}

o := addFlagsOptions{}
for _, opt := range opts {
opt(&o)
}

// Add all supported flags.
// Add flags with pflag deprecation remark for some klog flags.
packageFlags.VisitAll(func(f *flag.Flag) {
pf := pflag.PFlagFromGoFlag(f)
switch f.Name {
case "v", logsapi.LogFlushFreqFlagName:
case "v":
// unchanged, potentially skip it
if o.skipLoggingConfigurationFlags {
return
}
case logsapi.LogFlushFreqFlagName:
// unchanged, potentially skip it
if o.skipLoggingConfigurationFlags {
return
}
case "vmodule":
// TODO: see above
// pf.Usage += vmoduleUsage
if o.skipLoggingConfigurationFlags {
return
}
pf.Usage += vmoduleUsage
}
if fs.Lookup(pf.Name) == nil {
fs.AddFlag(pf)
default:
// deprecated, but not hidden
pf.Deprecated = deprecated
}
fs.AddFlag(pf)
})
}

Expand All @@ -119,16 +137,24 @@ func AddGoFlags(fs *flag.FlagSet, opts ...Option) {
packageFlags.VisitAll(func(f *flag.Flag) {
usage := f.Usage
switch f.Name {
case "v", logsapi.LogFlushFreqFlagName:
case "v":
// unchanged
if o.skipLoggingConfigurationFlags {
return
}
case logsapi.LogFlushFreqFlagName:
// unchanged
if o.skipLoggingConfigurationFlags {
return
}
case "vmodule":
// TODO: see above
// usage += vmoduleUsage
if o.skipLoggingConfigurationFlags {
return
}
usage += vmoduleUsage
default:
usage += " (DEPRECATED: " + deprecated + ")"
}
fs.Var(f.Value, f.Name, usage)
})
Expand Down
1 change: 1 addition & 0 deletions test/e2e_node/conformance/run_test.sh
Expand Up @@ -201,6 +201,7 @@ start_kubelet --kubeconfig "${KUBELET_KUBECONFIG}" \
--system-cgroups=/system \
--cgroup-root=/ \
--v=$log_level \
--logtostderr

wait_kubelet

Expand Down
2 changes: 1 addition & 1 deletion test/e2e_node/jenkins/conformance/conformance-jenkins.sh
Expand Up @@ -31,7 +31,7 @@ TIMEOUT=${TIMEOUT:-"45m"}
mkdir -p "${ARTIFACTS}"

go run test/e2e_node/runner/remote/run_remote.go --test-suite=conformance \
--vmodule=*=4 --ssh-env="gce" --ssh-user="$GCE_USER" \
--logtostderr --vmodule=*=4 --ssh-env="gce" --ssh-user="$GCE_USER" \
--zone="$GCE_ZONE" --project="$GCE_PROJECT" --hosts="$GCE_HOSTS" \
--images="$GCE_IMAGES" --image-project="$GCE_IMAGE_PROJECT" \
--image-config-file="$GCE_IMAGE_CONFIG_PATH" --cleanup="$CLEANUP" \
Expand Down
2 changes: 1 addition & 1 deletion test/e2e_node/remote/node_conformance.go
Expand Up @@ -181,7 +181,7 @@ func launchKubelet(host, workspace, results, testArgs, bearerToken string) error
return fmt.Errorf("failed to create kubelet pod manifest path %q: error - %v output - %q",
podManifestPath, err, output)
}
startKubeletCmd := fmt.Sprintf("./%s --run-kubelet-mode --node-name=%s"+
startKubeletCmd := fmt.Sprintf("./%s --run-kubelet-mode --logtostderr --node-name=%s"+
" --bearer-token=%s"+
" --report-dir=%s %s --kubelet-flags=--pod-manifest-path=%s > %s 2>&1",
conformanceTestBinary, host, bearerToken, results, testArgs, podManifestPath, filepath.Join(results, kubeletLauncherLog))
Expand Down
2 changes: 1 addition & 1 deletion test/e2e_node/remote/node_e2e.go
Expand Up @@ -200,7 +200,7 @@ func (n *NodeE2ERemote) RunTest(host, workspace, results, imageDesc, junitFilePr
klog.V(2).Infof("Starting tests on %q", host)
cmd := getSSHCommand(" && ",
fmt.Sprintf("cd %s", workspace),
fmt.Sprintf("timeout -k 30s %fs ./ginkgo %s ./e2e_node.test -- --system-spec-name=%s --system-spec-file=%s --extra-envs=%s --runtime-config=%s --v 4 --node-name=%s --report-dir=%s --report-prefix=%s --image-description=\"%s\" %s",
fmt.Sprintf("timeout -k 30s %fs ./ginkgo %s ./e2e_node.test -- --system-spec-name=%s --system-spec-file=%s --extra-envs=%s --runtime-config=%s --logtostderr --v 4 --node-name=%s --report-dir=%s --report-prefix=%s --image-description=\"%s\" %s",
timeout.Seconds(), ginkgoArgs, systemSpecName, systemSpecFile, extraEnvs, runtimeConfig, host, results, junitFilePrefix, imageDesc, testArgs),
)
return SSH(host, "sh", "-c", cmd)
Expand Down
4 changes: 2 additions & 2 deletions test/e2e_node/runner/remote/run_remote.go
Expand Up @@ -15,9 +15,9 @@ limitations under the License.
*/

// To run the node e2e tests remotely against one or more hosts on gce:
// $ go run run_remote.go --v 2 --ssh-env gce --hosts <comma separated hosts>
// $ go run run_remote.go --logtostderr --v 2 --ssh-env gce --hosts <comma separated hosts>
// To run the node e2e tests remotely against one or more images on gce and provision them:
// $ go run run_remote.go --v 2 --project <project> --zone <zone> --ssh-env gce --images <comma separated images>
// $ go run run_remote.go --logtostderr --v 2 --project <project> --zone <zone> --ssh-env gce --images <comma separated images>
package main

import (
Expand Down
2 changes: 1 addition & 1 deletion test/e2e_node/services/kubelet.go
Expand Up @@ -253,7 +253,7 @@ func (e *E2EServices) startKubelet(featureGates map[string]bool) (*server, error
cmdArgs = append(cmdArgs,
"--kubeconfig", kubeconfigPath,
"--root-dir", KubeletRootDirectory,
"--v", LogVerbosityLevel,
"--v", LogVerbosityLevel, "--logtostderr",
)

// Apply test framework feature gates by default. This could also be overridden
Expand Down
6 changes: 3 additions & 3 deletions test/integration/scheduler_perf/README.md
Expand Up @@ -32,7 +32,7 @@ How To Run

```shell
# In Kubernetes root path
make test-integration WHAT=./test/integration/scheduler_perf ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling"
make test-integration WHAT=./test/integration/scheduler_perf ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-alsologtostderr=false -logtostderr=false -run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling"
```

The benchmark suite runs all the tests specified under config/performance-config.yaml.
Expand All @@ -47,14 +47,14 @@ Otherwise, the golang benchmark framework will try to run a test more than once

```shell
# In Kubernetes root path
make test-integration WHAT=./test/integration/scheduler_perf ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling/SchedulingBasic/5000Nodes/5000InitPods/1000PodsToSchedule"
make test-integration WHAT=./test/integration/scheduler_perf ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-alsologtostderr=false -logtostderr=false -run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling/SchedulingBasic/5000Nodes/5000InitPods/1000PodsToSchedule"
```

To produce a cpu profile:

```shell
# In Kubernetes root path
make test-integration WHAT=./test/integration/scheduler_perf KUBE_TIMEOUT="-timeout=3600s" ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling -cpuprofile ~/cpu-profile.out"
make test-integration WHAT=./test/integration/scheduler_perf KUBE_TIMEOUT="-timeout=3600s" ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-alsologtostderr=false -logtostderr=false -run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling -cpuprofile ~/cpu-profile.out"
```

### How to configure benchmark tests
Expand Down

0 comments on commit e250f13

Please sign in to comment.