Skip to content

Commit

Permalink
Export Logger and NewLogger (istio#645)
Browse files Browse the repository at this point in the history
* Export Logger and NewLogger

* Fix test error

* Fix lint error
  • Loading branch information
irisdingbj authored and istio-testing committed Nov 28, 2019
1 parent 6a46e6d commit a9f8f28
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 44 deletions.
6 changes: 3 additions & 3 deletions cmd/mesh/manifest-apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func addManifestApplyFlags(cmd *cobra.Command, args *manifestApplyArgs) {
" The --wait flag must be set for this flag to apply")
cmd.PersistentFlags().BoolVarP(&args.wait, "wait", "w", false, "Wait, if set will wait until all Pods, Services, and minimum number of Pods "+
"of a Deployment are in a ready state before the command exits. It will wait for a maximum duration of --readiness-timeout seconds")
cmd.PersistentFlags().StringSliceVarP(&args.set, "set", "s", nil, setFlagHelpStr)
cmd.PersistentFlags().StringSliceVarP(&args.set, "set", "s", nil, SetFlagHelpStr)
}

func manifestApplyCmd(rootArgs *rootArgs, maArgs *manifestApplyArgs) *cobra.Command {
Expand All @@ -65,7 +65,7 @@ func manifestApplyCmd(rootArgs *rootArgs, maArgs *manifestApplyArgs) *cobra.Comm
Long: "The apply subcommand generates an Istio install manifest and applies it to a cluster.",
Args: cobra.ExactArgs(0),
RunE: func(cmd *cobra.Command, args []string) error {
l := newLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())
l := NewLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())
// Warn users if they use `manifest apply` without any config args.
if maArgs.inFilename == "" && len(maArgs.set) == 0 && !maArgs.skipConfirmation {
if !confirm("This will install the default Istio profile into the cluster. Proceed? (y/N)", cmd.OutOrStdout()) {
Expand All @@ -77,7 +77,7 @@ func manifestApplyCmd(rootArgs *rootArgs, maArgs *manifestApplyArgs) *cobra.Comm
}}
}

func manifestApply(args *rootArgs, maArgs *manifestApplyArgs, l *logger) error {
func manifestApply(args *rootArgs, maArgs *manifestApplyArgs, l *Logger) error {
if err := configLogs(args.logToStdErr); err != nil {
return fmt.Errorf("could not configure logs: %s", err)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/mesh/manifest-common.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var (
)

func genApplyManifests(setOverlay []string, inFilename string, force bool, dryRun bool, verbose bool,
kubeConfigPath string, context string, waitTimeout time.Duration, l *logger) error {
kubeConfigPath string, context string, waitTimeout time.Duration, l *Logger) error {
overlayFromSet, err := MakeTreeFromSetList(setOverlay, force, l)
if err != nil {
return fmt.Errorf("failed to generate tree from the set overlay, error: %v", err)
Expand Down Expand Up @@ -93,7 +93,7 @@ func genApplyManifests(setOverlay []string, inFilename string, force bool, dryRu
}

// GenManifests generate manifest from input file and setOverLay
func GenManifests(inFilename string, setOverlayYAML string, force bool, l *logger) (name.ManifestMap, error) {
func GenManifests(inFilename string, setOverlayYAML string, force bool, l *Logger) (name.ManifestMap, error) {
mergedYAML, err := genProfile(false, inFilename, "", setOverlayYAML, "", force, l)
if err != nil {
return nil, err
Expand Down Expand Up @@ -154,7 +154,7 @@ func fetchInstallPackageFromURL(mergedICPS *v1alpha2.IstioControlPlaneSpec) erro
}

// MakeTreeFromSetList creates a YAML tree from a string slice containing key-value pairs in the format key=value.
func MakeTreeFromSetList(setOverlay []string, force bool, l *logger) (string, error) {
func MakeTreeFromSetList(setOverlay []string, force bool, l *Logger) (string, error) {
if len(setOverlay) == 0 {
return "", nil
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/mesh/manifest-generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type manifestGenerateArgs struct {
func addManifestGenerateFlags(cmd *cobra.Command, args *manifestGenerateArgs) {
cmd.PersistentFlags().StringVarP(&args.inFilename, "filename", "f", "", filenameFlagHelpStr)
cmd.PersistentFlags().StringVarP(&args.outFilename, "output", "o", "", "Manifest output directory path")
cmd.PersistentFlags().StringSliceVarP(&args.set, "set", "s", nil, setFlagHelpStr)
cmd.PersistentFlags().StringSliceVarP(&args.set, "set", "s", nil, SetFlagHelpStr)
cmd.PersistentFlags().BoolVar(&args.force, "force", false, "Proceed even with validation errors")
}

Expand All @@ -56,13 +56,13 @@ func manifestGenerateCmd(rootArgs *rootArgs, mgArgs *manifestGenerateArgs) *cobr
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
l := newLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())
l := NewLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())
return manifestGenerate(rootArgs, mgArgs, l)
}}

}

func manifestGenerate(args *rootArgs, mgArgs *manifestGenerateArgs, l *logger) error {
func manifestGenerate(args *rootArgs, mgArgs *manifestGenerateArgs, l *Logger) error {
if err := configLogs(args.logToStdErr); err != nil {
return fmt.Errorf("could not configure logs: %s", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/mesh/manifest-generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func TestLDFlags(t *testing.T) {
}()
version.DockerInfo.Hub = "testHub"
version.DockerInfo.Tag = "testTag"
l := newLogger(true, os.Stdout, os.Stderr)
l := NewLogger(true, os.Stdout, os.Stderr)
_, icps, err := genICPS("", "default", "", true, l)
if err != nil {
t.Fatal(err)
Expand Down
8 changes: 4 additions & 4 deletions cmd/mesh/manifest-migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func manifestMigrateCmd(rootArgs *rootArgs, mmArgs *manifestMigrateArgs) *cobra.
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
l := newLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())
l := NewLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())

if len(args) == 0 {
return migrateFromClusterConfig(rootArgs, mmArgs, l)
Expand All @@ -71,7 +71,7 @@ func valueFileFilter(path string) bool {
}

// migrateFromFiles handles migration for local values.yaml files
func migrateFromFiles(rootArgs *rootArgs, args []string, l *logger) error {
func migrateFromFiles(rootArgs *rootArgs, args []string, l *Logger) error {
initLogsOrExit(rootArgs)
value, err := util.ReadFilesWithFilter(args[0], valueFileFilter)
if err != nil {
Expand All @@ -85,7 +85,7 @@ func migrateFromFiles(rootArgs *rootArgs, args []string, l *logger) error {
}

// translateFunc translates the input values and output the result
func translateFunc(values []byte, l *logger) error {
func translateFunc(values []byte, l *Logger) error {
ts, err := translate.NewReverseTranslator(binversion.OperatorBinaryVersion.MinorVersion)
if err != nil {
return fmt.Errorf("error creating values.yaml translator: %s", err)
Expand Down Expand Up @@ -113,7 +113,7 @@ func translateFunc(values []byte, l *logger) error {
}

// migrateFromClusterConfig handles migration for in cluster config.
func migrateFromClusterConfig(rootArgs *rootArgs, mmArgs *manifestMigrateArgs, l *logger) error {
func migrateFromClusterConfig(rootArgs *rootArgs, mmArgs *manifestMigrateArgs, l *Logger) error {
initLogsOrExit(rootArgs)

l.logAndPrint("translating in cluster specs\n")
Expand Down
8 changes: 4 additions & 4 deletions cmd/mesh/manifest-versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ func manifestVersionsCmd(rootArgs *rootArgs, versionsArgs *manifestVersionsArgs)
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
l := newLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())
l := NewLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())
return manifestVersions(rootArgs, versionsArgs, l)
}}
}

func manifestVersions(args *rootArgs, mvArgs *manifestVersionsArgs, l *logger) error {
func manifestVersions(args *rootArgs, mvArgs *manifestVersionsArgs, l *Logger) error {
initLogsOrExit(args)

myVersionMap, err := getVersionCompatibleMap(mvArgs.versionsURI, binversion.OperatorBinaryGoVersion, l)
Expand All @@ -84,7 +84,7 @@ func manifestVersions(args *rootArgs, mvArgs *manifestVersionsArgs, l *logger) e
}

func getVersionCompatibleMap(versionsURI string, binVersion *goversion.Version,
l *logger) (*version.CompatibilityMapping, error) {
l *Logger) (*version.CompatibilityMapping, error) {
var b []byte
var err error

Expand All @@ -110,7 +110,7 @@ func getVersionCompatibleMap(versionsURI string, binVersion *goversion.Version,
return myVersionMap, nil
}

func loadCompatibleMapFile(versionsURI string, l *logger) ([]byte, error) {
func loadCompatibleMapFile(versionsURI string, l *Logger) ([]byte, error) {
var err error
if util.IsHTTPURL(versionsURI) {
if b, err := httprequest.Get(versionsURI); err == nil {
Expand Down
4 changes: 2 additions & 2 deletions cmd/mesh/manifest-versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestGetVersionCompatibleMap(t *testing.T) {
type args struct {
versionsURI string
binVersion *goversion.Version
l *logger
l *Logger
}

testDataDir = filepath.Join(repoRootDir, "cmd/mesh/testdata/manifest-versions")
Expand All @@ -43,7 +43,7 @@ func TestGetVersionCompatibleMap(t *testing.T) {
goVerNonexistent, _ := goversion.NewVersion("0.0.999")
goVer133, _ := goversion.NewVersion("1.3.3")

l := newLogger(true, os.Stdout, os.Stderr)
l := NewLogger(true, os.Stdout, os.Stderr)

b, err := ioutil.ReadFile(operatorVersionsFilePath)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions cmd/mesh/profile-common.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import (
// ones that are compiled in. If it does, the starting point will be the base and profile YAMLs at that file path.
// Otherwise it will be the compiled in profile YAMLs.
// In step 3, the remaining fields in the same user overlay are applied on the resulting profile base.
func genICPS(inFilename, profile, setOverlayYAML string, force bool, l *logger) (string, *v1alpha2.IstioControlPlaneSpec, error) {
func genICPS(inFilename, profile, setOverlayYAML string, force bool, l *Logger) (string, *v1alpha2.IstioControlPlaneSpec, error) {
overlayYAML := ""
var overlayICPS *v1alpha2.IstioControlPlaneSpec
set := make(map[string]interface{})
Expand Down Expand Up @@ -128,7 +128,7 @@ func genICPS(inFilename, profile, setOverlayYAML string, force bool, l *logger)
return finalYAML, finalICPS, nil
}

func genProfile(helmValues bool, inFilename, profile, setOverlayYAML, configPath string, force bool, l *logger) (string, error) {
func genProfile(helmValues bool, inFilename, profile, setOverlayYAML, configPath string, force bool, l *Logger) (string, error) {
finalYAML, finalICPS, err := genICPS(inFilename, profile, setOverlayYAML, force, l)
if err != nil {
return "", err
Expand Down Expand Up @@ -175,7 +175,7 @@ func unmarshalAndValidateICP(crYAML string, force bool) (*v1alpha2.IstioControlP
return icps, icpsYAML, nil
}

func unmarshalAndValidateICPS(icpsYAML string, force bool, l *logger) (*v1alpha2.IstioControlPlaneSpec, error) {
func unmarshalAndValidateICPS(icpsYAML string, force bool, l *Logger) (*v1alpha2.IstioControlPlaneSpec, error) {
icps := &v1alpha2.IstioControlPlaneSpec{}
if err := util.UnmarshalWithJSONPB(icpsYAML, icps); err != nil {
return nil, fmt.Errorf("could not unmarshal the merged YAML: %s\n\nYAML:\n%s", err, icpsYAML)
Expand Down
4 changes: 2 additions & 2 deletions cmd/mesh/profile-dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ func profileDumpCmd(rootArgs *rootArgs, pdArgs *profileDumpArgs) *cobra.Command
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
l := newLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())
l := NewLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.ErrOrStderr())
return profileDump(args, rootArgs, pdArgs, l)
}}

}

func profileDump(args []string, rootArgs *rootArgs, pdArgs *profileDumpArgs, l *logger) error {
func profileDump(args []string, rootArgs *rootArgs, pdArgs *profileDumpArgs, l *Logger) error {
initLogsOrExit(rootArgs)

if len(args) == 1 && pdArgs.inFilename != "" {
Expand Down
2 changes: 1 addition & 1 deletion cmd/mesh/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
)

const (
setFlagHelpStr = `Set a value in IstioControlPlane CustomResource. e.g. --set policy.enabled=true.
SetFlagHelpStr = `Set a value in IstioControlPlane CustomResource. e.g. --set policy.enabled=true.
Overrides the corresponding path value in the selected profile or passed through IstioControlPlane CR
customization file`
skipConfirmationFlagHelpStr = `skipConfirmation determines whether the user is prompted for confirmation.
Expand Down
23 changes: 12 additions & 11 deletions cmd/mesh/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,25 @@ func configLogs(logToStdErr bool) error {
return log.Configure(opt)
}

type logger struct {
//Logger is the struct used for mesh command
type Logger struct {
logToStdErr bool
stdOut io.Writer
stdErr io.Writer
}

// newLogger creates a new logger and returns a pointer to it.
// NewLogger creates a new logger and returns a pointer to it.
// stdOut and stdErr can be used to capture output for testing.
func newLogger(logToStdErr bool, stdOut, stdErr io.Writer) *logger {
return &logger{
func NewLogger(logToStdErr bool, stdOut, stdErr io.Writer) *Logger {
return &Logger{
logToStdErr: logToStdErr,
stdOut: stdOut,
stdErr: stdErr,
}
}

// TODO: this really doesn't belong here. Figure out if it's generally needed and possibly move to istio.io/pkg/log.
func (l *logger) logAndPrint(v ...interface{}) {
func (l *Logger) logAndPrint(v ...interface{}) {
if len(v) == 0 {
return
}
Expand All @@ -84,7 +85,7 @@ func (l *logger) logAndPrint(v ...interface{}) {
}
}

func (l *logger) logAndError(v ...interface{}) {
func (l *Logger) logAndError(v ...interface{}) {
if len(v) == 0 {
return
}
Expand All @@ -96,7 +97,7 @@ func (l *logger) logAndError(v ...interface{}) {
}
}

func (l *logger) logAndPrintf(format string, a ...interface{}) {
func (l *Logger) logAndPrintf(format string, a ...interface{}) {
s := fmt.Sprintf(format, a...)
if !l.logToStdErr {
l.print(s + "\n")
Expand All @@ -105,7 +106,7 @@ func (l *logger) logAndPrintf(format string, a ...interface{}) {
}
}

func (l *logger) logAndErrorf(format string, a ...interface{}) {
func (l *Logger) logAndErrorf(format string, a ...interface{}) {
s := fmt.Sprintf(format, a...)
if !l.logToStdErr {
l.printErr(s + "\n")
Expand All @@ -114,16 +115,16 @@ func (l *logger) logAndErrorf(format string, a ...interface{}) {
}
}

func (l *logger) logAndFatalf(format string, a ...interface{}) {
func (l *Logger) logAndFatalf(format string, a ...interface{}) {
l.logAndErrorf(format, a...)
os.Exit(-1)
}

func (l *logger) print(s string) {
func (l *Logger) print(s string) {
_, _ = l.stdOut.Write([]byte(s))
}

func (l *logger) printErr(s string) {
func (l *Logger) printErr(s string) {
_, _ = l.stdErr.Write([]byte(s))
}

Expand Down
14 changes: 7 additions & 7 deletions cmd/mesh/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func UpgradeCmd() *cobra.Command {
"traffic may be disrupted during upgrade. Please ensure PodDisruptionBudgets " +
"are defined to maintain service continuity.",
RunE: func(cmd *cobra.Command, args []string) (e error) {
l := newLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.OutOrStderr())
l := NewLogger(rootArgs.logToStdErr, cmd.OutOrStdout(), cmd.OutOrStderr())
initLogsOrExit(rootArgs)
err := upgrade(rootArgs, macArgs, l)
if err != nil {
Expand All @@ -119,7 +119,7 @@ func UpgradeCmd() *cobra.Command {
}

// upgrade is the main function for Upgrade command
func upgrade(rootArgs *rootArgs, args *upgradeArgs, l *logger) (err error) {
func upgrade(rootArgs *rootArgs, args *upgradeArgs, l *Logger) (err error) {
l.logAndPrintf("Client - istioctl version: %s\n", opversion.OperatorVersionString)
args.inFilename = strings.TrimSpace(args.inFilename)

Expand Down Expand Up @@ -240,7 +240,7 @@ func upgrade(rootArgs *rootArgs, args *upgradeArgs, l *logger) (err error) {
}

// checkUpgradeValues checks the upgrade eligibility by comparing the current values with the target values
func checkUpgradeValues(curValues, tarValues, ignoreValues string, l *logger) {
func checkUpgradeValues(curValues, tarValues, ignoreValues string, l *Logger) {
diff := compare.YAMLCmpWithIgnore(curValues, tarValues, nil, ignoreValues)
if diff == "" {
l.logAndPrintf("Upgrade check: Values unchanged. The target values are identical to the current values.\n")
Expand All @@ -251,7 +251,7 @@ func checkUpgradeValues(curValues, tarValues, ignoreValues string, l *logger) {
}

// waitForConfirmation waits for user's confirmation if skipConfirmation is not set
func waitForConfirmation(skipConfirmation bool, l *logger) {
func waitForConfirmation(skipConfirmation bool, l *Logger) {
if skipConfirmation {
return
}
Expand Down Expand Up @@ -291,7 +291,7 @@ func readValuesFromInjectorConfigMap(kubeClient manifest.ExecClient, istioNamesp
}

// checkSupportedVersions checks if the upgrade cur -> tar is supported by the tool
func checkSupportedVersions(cur, tar, versionsURI string, l *logger) error {
func checkSupportedVersions(cur, tar, versionsURI string, l *Logger) error {
tarGoVersion, err := goversion.NewVersion(tar)
if err != nil {
return fmt.Errorf("failed to parse the target version: %v", tar)
Expand All @@ -315,7 +315,7 @@ func checkSupportedVersions(cur, tar, versionsURI string, l *logger) error {
}

// retrieveControlPlaneVersion retrieves the version number from the Istio control plane
func retrieveControlPlaneVersion(kubeClient manifest.ExecClient, istioNamespace string, l *logger) (string, error) {
func retrieveControlPlaneVersion(kubeClient manifest.ExecClient, istioNamespace string, l *Logger) (string, error) {
cv, e := kubeClient.GetIstioVersions(istioNamespace)
if e != nil {
return "", fmt.Errorf("failed to retrieve Istio control plane version, error: %v", e)
Expand All @@ -339,7 +339,7 @@ func retrieveControlPlaneVersion(kubeClient manifest.ExecClient, istioNamespace

// waitUpgradeComplete waits for the upgrade to complete by periodically comparing the current component version
// to the target version.
func waitUpgradeComplete(kubeClient manifest.ExecClient, istioNamespace string, targetVer string, l *logger) error {
func waitUpgradeComplete(kubeClient manifest.ExecClient, istioNamespace string, targetVer string, l *Logger) error {
for i := 1; i <= upgradeWaitCheckVerMaxAttempts; i++ {
sleepSeconds(upgradeWaitSecCheckVerPerLoop)
cv, e := kubeClient.GetIstioVersions(istioNamespace)
Expand Down

0 comments on commit a9f8f28

Please sign in to comment.