config: change LogColorOverride to "string" (#212)

* config: change LogColorOverride to "string" Fix #211. Signed-off-by: Gyuho Lee <leegyuho@amazon.com> * CHANGELOG: update "LogColorOverride" change Signed-off-by: Gyuho Lee <leegyuho@amazon.com>
aws · Apr 26, 2021 · b6e86b1 · b6e86b1
1 parent 516c9e2
commit b6e86b1
Show file tree

Hide file tree

Showing 12 changed files with 162 additions and 142 deletions.
diff --git a/CHANGELOG/CHANGELOG-1.5.md b/CHANGELOG/CHANGELOG-1.5.md
@@ -8,6 +8,12 @@
 
 - Initial commit for [`"github.com/aws/aws-k8s-tester/client"`](https://github.com/aws/aws-k8s-tester/commit/de76767f6e0972d35370457dc67dd4959b9e638f).
 
+### `ec2config`
+
+- Change [`LogColorOverride` field from `bool` to `string`](https://github.com/aws/aws-k8s-tester/pull/212).
+  - Fix https://github.com/aws/aws-k8s-tester/issues/211.
+  - If not empty, now `terminal.IsColor` is not run.
+
 ### `eks`
 
 - Add [`eks/stresser2`](https://github.com/aws/aws-k8s-tester/pull/206).
@@ -17,6 +23,9 @@
 - Add [`AWS_K8S_TESTER_EKS_ADD_ON_CLUSTER_LOADER_LOCAL_CL2_SCHEDULER_THROUGHPUT_THRESHOLD`](https://github.com/aws/aws-k8s-tester/pull/208).
 - Add [`AWS_K8S_TESTER_EKS_ADD_ON_CLUSTER_LOADER_REMOTE_CL2_SCHEDULER_THROUGHPUT_THRESHOLD`](https://github.com/aws/aws-k8s-tester/pull/208).
 - Ignore error [for an unknown region](https://github.com/aws/aws-k8s-tester/pull/204).
+- Change [`LogColorOverride` field from `bool` to `string`](https://github.com/aws/aws-k8s-tester/pull/212).
+  - Fix https://github.com/aws/aws-k8s-tester/issues/211.
+  - If not empty, now `terminal.IsColor` is not run.
 
 ### `k8s-tester`
 

diff --git a/ec2/ec2.go b/ec2/ec2.go
@@ -15,7 +15,6 @@ import (
 	"github.com/aws/aws-k8s-tester/ec2config"
 	pkgaws "github.com/aws/aws-k8s-tester/pkg/aws"
 	"github.com/aws/aws-k8s-tester/pkg/logutil"
-	"github.com/aws/aws-k8s-tester/pkg/terminal"
 	"github.com/aws/aws-k8s-tester/version"
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/session"
@@ -37,7 +36,6 @@ import (
 	"github.com/aws/aws-sdk-go/service/ssm/ssmiface"
 	"github.com/aws/aws-sdk-go/service/sts"
 	"github.com/dustin/go-humanize"
-	"github.com/mitchellh/colorstring"
 	"go.uber.org/zap"
 )
 
@@ -81,21 +79,7 @@ func New(cfg *ec2config.Config) (*Tester, error) {
 	if err != nil {
 		return nil, err
 	}
-	lg.Info("set up log writer and file", zap.Strings("outputs", cfg.LogOutputs))
-
-	isColor := cfg.LogColor
-	co, cerr := terminal.IsColor()
-	if cerr == nil {
-		lg.Info("requested output in color", zap.String("output", co), zap.Error(cerr))
-		colorstring.Printf("\n\n[light_green]HELLO COLOR\n\n")
-		isColor = true
-	} else if !cfg.LogColorOverride {
-		lg.Warn("requested output in color but not supported; overriding", zap.String("output", co), zap.Error(cerr))
-		isColor = false
-	} else {
-		lg.Info("requested output color", zap.Bool("is-color", isColor), zap.String("output", co), zap.Error(cerr))
-	}
-	cfg.LogColor = isColor
+	lg.Info("set up log writer and file", zap.Strings("outputs", cfg.LogOutputs), zap.Bool("is-color", cfg.LogColor))
 	cfg.Sync()
 
 	fmt.Fprintf(logWriter, cfg.Colorize("\n\n[yellow]*********************************\n"))

diff --git a/ec2config/README.md b/ec2config/README.md
@@ -17,7 +17,7 @@
 | AWS_K8S_TESTER_EC2_AWS_IAM_ROLE_ARN                    | read-only "true"  | *ec2config.Config.AWSIAMRoleARN                   | string                   |
 | AWS_K8S_TESTER_EC2_AWS_CREDENTIAL_PATH                 | read-only "true"  | *ec2config.Config.AWSCredentialPath               | string                   |
 | AWS_K8S_TESTER_EC2_LOG_COLOR                           | read-only "false" | *ec2config.Config.LogColor                        | bool                     |
-| AWS_K8S_TESTER_EC2_LOG_COLOR_OVERRIDE                  | read-only "false" | *ec2config.Config.LogColorOverride                | bool                     |
+| AWS_K8S_TESTER_EC2_LOG_COLOR_OVERRIDE                  | read-only "false" | *ec2config.Config.LogColorOverride                | string                   |
 | AWS_K8S_TESTER_EC2_LOG_LEVEL                           | read-only "false" | *ec2config.Config.LogLevel                        | string                   |
 | AWS_K8S_TESTER_EC2_LOG_OUTPUTS                         | read-only "false" | *ec2config.Config.LogOutputs                      | []string                 |
 | AWS_K8S_TESTER_EC2_ON_FAILURE_DELETE                   | read-only "false" | *ec2config.Config.OnFailureDelete                 | bool                     |

diff --git a/ec2config/config.go b/ec2config/config.go
@@ -87,10 +87,14 @@ type Config struct {
 
 	// LogColor is true to output logs in color.
 	LogColor bool `json:"log-color"`
-	// LogColorOverride is true to use "LogColor" setting
-	// even if the current terminal does not support color outputs.
-	// Useful to output in color in HTML based log outputs (e.g. Prow).
-	LogColorOverride bool `json:"log-color-override"`
+	// LogColorOverride is not empty to override "LogColor" setting.
+	// If not empty, the automatic color check is not even run and use this value instead.
+	// For instance, github action worker might not support color device,
+	// thus exiting color check with the exit code 1.
+	// Useful to output in color in HTML based log outputs (e.g., Prow).
+	// Useful to skip terminal color check when there is no color device (e.g., Github action worker).
+	LogColorOverride string `json:"log-color-override"`
+
 	// LogLevel configures log level. Only supports debug, info, warn, error, panic, or fatal. Default 'info'.
 	LogLevel string `json:"log-level"`
 	// LogOutputs is a list of log outputs. Valid values are 'default', 'stderr', 'stdout', or file names.

diff --git a/ec2config/default.yaml b/ec2config/default.yaml
@@ -1,9 +1,9 @@
 asgs:
-  ec2-2021041219-integritynoq-asg:
+  ec2-2021042610-flowerp40w88-asg:
     ami-type: AL2_x86_64
     asg-cfn-stack-id: ""
-    asg-cfn-stack-yaml-path: /home/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.asg.cfn.ec2-2021041219-integritynoq-asg.yaml
-    asg-cfn-stack-yaml-s3-key: ec2-2021041219-integritynoq/default.asg.cfn.ec2-2021041219-integritynoq-asg.yaml
+    asg-cfn-stack-yaml-path: /Users/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.asg.cfn.ec2-2021042610-flowerp40w88-asg.yaml
+    asg-cfn-stack-yaml-s3-key: ec2-2021042610-flowerp40w88/default.asg.cfn.ec2-2021042610-flowerp40w88-asg.yaml
     asg-desired-capacity: 1
     asg-max-size: 1
     asg-min-size: 1
@@ -13,12 +13,12 @@ asgs:
     instance-types:
     - c5.xlarge
     logs: null
-    name: ec2-2021041219-integritynoq-asg
+    name: ec2-2021042610-flowerp40w88-asg
     remote-access-user-name: ec2-user
     ssm-document-cfn-stack-id: ""
     ssm-document-cfn-stack-name: ""
-    ssm-document-cfn-stack-yaml-path: /home/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.ssm.cfn.ec2-2021041219-integritynoq-asg.yaml
-    ssm-document-cfn-stack-yaml-s3-key: ec2-2021041219-integritynoq/default.ssm.cfn.ec2-2021041219-integritynoq-asg.yaml
+    ssm-document-cfn-stack-yaml-path: /Users/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.ssm.cfn.ec2-2021042610-flowerp40w88-asg.yaml
+    ssm-document-cfn-stack-yaml-s3-key: ec2-2021042610-flowerp40w88/default.ssm.cfn.ec2-2021042610-flowerp40w88-asg.yaml
     ssm-document-command-ids: null
     ssm-document-commands: ""
     ssm-document-create: false
@@ -40,44 +40,44 @@ asgs:
       took-string: ""
     volume-size: 40
 asgs-fetch-logs: true
-asgs-logs-dir: /home/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/ec2-2021041219-integritynoq-logs-remote
+asgs-logs-dir: /Users/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/ec2-2021042610-flowerp40w88-logs-remote
 aws-account-id: ""
 aws-credential-path: ""
 aws-iam-role-arn: ""
 aws-user-id: ""
-config-path: /home/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.yaml
+config-path: /Users/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.yaml
 dhcp-options-domain-name: ""
 dhcp-options-domain-name-servers: null
 log-color: true
-log-color-override: false
+log-color-override: ""
 log-level: info
 log-outputs:
 - stderr
-- /home/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.yaml.log
-name: ec2-2021041219-integritynoq
+- /Users/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.yaml.log
+name: ec2-2021042610-flowerp40w88
 on-failure-delete: true
 on-failure-delete-wait-seconds: 120
 partition: aws
 private-subnet-ids: null
 public-subnet-ids: null
 region: us-west-2
-remote-access-commands-output-path: /home/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.ssh.sh
+remote-access-commands-output-path: /Users/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.ssh.sh
 remote-access-key-create: true
-remote-access-key-name: ec2-2021041219-integritynoq-key
-remote-access-private-key-path: /tmp/duneroi6ln.insecure.key
+remote-access-key-name: ec2-2021042610-flowerp40w88-key
+remote-access-private-key-path: /var/folders/xt/szh_n5x154x2hyhj0zkpd3qr0000gn/T/mountaincz.insecure.key
 role-arn: ""
 role-cfn-stack-id: ""
-role-cfn-stack-yaml-path: /home/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.role.cfn.yaml
-role-cfn-stack-yaml-s3-key: ec2-2021041219-integritynoq/default.role.cfn.yaml
+role-cfn-stack-yaml-path: /Users/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.role.cfn.yaml
+role-cfn-stack-yaml-s3-key: ec2-2021042610-flowerp40w88/default.role.cfn.yaml
 role-create: true
 role-managed-policy-arns: null
-role-name: ec2-2021041219-integritynoq-role
+role-name: ec2-2021042610-flowerp40w88-role
 role-service-principals: null
 s3-bucket-create: true
 s3-bucket-create-keep: true
 s3-bucket-lifecycle-expiration-days: 0
-s3-bucket-name: ec2-2021041219-integritynoq-s3-bucket
-s3-dir: ec2-2021041219-integritynoq
+s3-bucket-name: ec2-2021042610-flowerp40w88-s3-bucket
+s3-dir: ec2-2021042610-flowerp40w88
 security-group-id: ""
 ssh-ingress-ipv4-range: ""
 status: null
@@ -99,7 +99,7 @@ time-frame-delete:
 total-nodes: 1
 up: false
 vpc-cfn-stack-id: ""
-vpc-cfn-stack-yaml-path: /home/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.vpc.cfn.yaml
-vpc-cfn-stack-yaml-s3-key: ec2-2021041219-integritynoq/default.vpc.cfn.yaml
+vpc-cfn-stack-yaml-path: /Users/leegyuho/go/src/github.com/aws/aws-k8s-tester/ec2config/default.vpc.cfn.yaml
+vpc-cfn-stack-yaml-s3-key: ec2-2021042610-flowerp40w88/default.vpc.cfn.yaml
 vpc-create: true
 vpc-id: ""
diff --git a/ec2config/env_test.go b/ec2config/env_test.go
@@ -58,8 +58,8 @@ func TestEnv(t *testing.T) {
 	if cfg.LogColor {
 		t.Fatalf("unexpected cfg.LogColor %v", cfg.LogColor)
 	}
-	if !cfg.LogColorOverride {
-		t.Fatalf("unexpected cfg.LogColorOverride %v", cfg.LogColorOverride)
+	if cfg.LogColorOverride != "true" {
+		t.Fatalf("unexpected LogColorOverride %q", cfg.LogColorOverride)
 	}
 	if !cfg.S3BucketCreate {
 		t.Fatalf("unexpected cfg.S3BucketCreate %v", cfg.S3BucketCreate)

diff --git a/ec2config/validate-defaults.go b/ec2config/validate-defaults.go
@@ -7,6 +7,7 @@ import (
 	"path"
 	"path/filepath"
 	"regexp"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -38,7 +39,9 @@ func NewDefault() *Config {
 		ConfigPath:                     "",
 		RemoteAccessCommandsOutputPath: "",
 
-		LogColor: true,
+		LogColor:         true,
+		LogColorOverride: "",
+
 		LogLevel: logutil.DefaultLogLevel,
 		// default, stderr, stdout, or file name
 		// log file named with cluster name will be added automatically
@@ -130,10 +133,22 @@ func (cfg *Config) validateConfig() error {
 		return fmt.Errorf("region %q for partition %q not found in %+v", cfg.Region, cfg.Partition, regions)
 	}
 
-	_, cerr := terminal.IsColor()
-	if cfg.LogColor && !cfg.LogColorOverride && cerr != nil {
-		cfg.LogColor = false
+	if cfg.LogColorOverride == "" {
+		_, cerr := terminal.IsColor()
+		if cfg.LogColor && cerr != nil {
+			cfg.LogColor = false
+			fmt.Fprintf(os.Stderr, "[WARN] LogColor is set to 'false' due to error %+v", cerr)
+		}
+	} else {
+		// non-empty override, don't even run "terminal.IsColor"
+		ov, perr := strconv.ParseBool(cfg.LogColorOverride)
+		if perr != nil {
+			return fmt.Errorf("failed to parse LogColorOverride %q (%v)", cfg.LogColorOverride, perr)
+		}
+		cfg.LogColor = ov
+		fmt.Fprintf(os.Stderr, "[WARN] LogColor is overwritten with %q", cfg.LogColorOverride)
 	}
+
 	if len(cfg.LogOutputs) == 0 {
 		return errors.New("LogOutputs is not empty")
 	}

diff --git a/eks/eks.go b/eks/eks.go
@@ -76,7 +76,6 @@ import (
 	"github.com/aws/aws-k8s-tester/pkg/httputil"
 	k8s_client "github.com/aws/aws-k8s-tester/pkg/k8s-client"
 	"github.com/aws/aws-k8s-tester/pkg/logutil"
-	"github.com/aws/aws-k8s-tester/pkg/terminal"
 	"github.com/aws/aws-k8s-tester/pkg/user"
 	"github.com/aws/aws-k8s-tester/version"
 	aws_v2 "github.com/aws/aws-sdk-go-v2/aws"
@@ -117,7 +116,6 @@ import (
 	"github.com/aws/aws-sdk-go/service/ssm/ssmiface"
 	"github.com/aws/aws-sdk-go/service/sts"
 	"github.com/dustin/go-humanize"
-	"github.com/mitchellh/colorstring"
 	"go.uber.org/zap"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog"
@@ -211,21 +209,7 @@ func New(cfg *eksconfig.Config) (ts *Tester, err error) {
 		return nil, err
 	}
 	_ = zap.ReplaceGlobals(lg)
-	lg.Info("set up log writer and file", zap.Strings("outputs", cfg.LogOutputs))
-
-	isColor := cfg.LogColor
-	co, cerr := terminal.IsColor()
-	if cerr == nil {
-		lg.Info("requested output in color", zap.String("output", co), zap.Error(cerr))
-		colorstring.Printf("\n\n[light_green]HELLO COLOR\n\n")
-		isColor = true
-	} else if !cfg.LogColorOverride {
-		lg.Warn("requested output in color but not supported; overriding", zap.String("output", co), zap.Error(cerr))
-		isColor = false
-	} else {
-		lg.Info("requested output color", zap.Bool("is-color", isColor), zap.String("output", co), zap.Error(cerr))
-	}
-	cfg.LogColor = isColor
+	lg.Info("set up log writer and file", zap.Strings("outputs", cfg.LogOutputs), zap.Bool("is-color", cfg.LogColor))
 	cfg.Sync()
 
 	colorize := cfg.Colorize