From a2d70f3c1c9eb34f855267ab88893bd83145f27f Mon Sep 17 00:00:00 2001 From: Pedro Coutinho Date: Wed, 4 Aug 2021 09:36:19 -0700 Subject: [PATCH 1/2] Refactor version mismatch checking to not need argument parsing Refactor version mismatch checking to run after argument parsing in the applicable commands, dismissing the need for its own fragile argument parsing. Add --allow-version-mismatch arg to every command (no-op when not applicable). Adjust version mismatch st test accordingly. --- Makefile | 3 +- calicoctl/calicoctl.go | 25 ++-- calicoctl/commands/apply.go | 33 ++--- calicoctl/commands/common/resources.go | 5 + calicoctl/commands/common/version_mismatch.go | 82 +++++++++++ calicoctl/commands/convert.go | 7 +- calicoctl/commands/create.go | 37 ++--- .../commands/datastore/migrate/export.go | 18 ++- .../commands/datastore/migrate/import.go | 17 ++- calicoctl/commands/datastore/migrate/lock.go | 17 ++- .../commands/datastore/migrate/unlock.go | 17 ++- calicoctl/commands/delete.go | 39 +++--- calicoctl/commands/get.go | 7 +- calicoctl/commands/ipam/check.go | 24 ++-- calicoctl/commands/ipam/configure.go | 9 +- calicoctl/commands/ipam/release.go | 23 ++-- calicoctl/commands/ipam/show.go | 26 ++-- calicoctl/commands/label.go | 9 +- calicoctl/commands/node/checksystem.go | 10 +- calicoctl/commands/node/diags.go | 11 +- calicoctl/commands/node/run.go | 9 +- calicoctl/commands/node/status.go | 7 +- calicoctl/commands/patch.go | 29 ++-- calicoctl/commands/replace.go | 33 ++--- calicoctl/commands/version.go | 127 ++---------------- tests/st/calicoctl/test_flags.py | 13 +- 26 files changed, 346 insertions(+), 291 deletions(-) create mode 100644 calicoctl/commands/common/version_mismatch.go diff --git a/Makefile b/Makefile index 8827864b5..9af7e01ad 100644 --- a/Makefile +++ b/Makefile @@ -59,7 +59,8 @@ TEST_CONTAINER_NAME ?= calico/test CALICOCTL_GIT_REVISION?=$(shell git rev-parse --short HEAD) LDFLAGS=-ldflags "-X $(PACKAGE_NAME)/v3/calicoctl/commands.VERSION=$(GIT_VERSION) \ - -X $(PACKAGE_NAME)/v3/calicoctl/commands.GIT_REVISION=$(CALICOCTL_GIT_REVISION) -s -w" + -X $(PACKAGE_NAME)/v3/calicoctl/commands.GIT_REVISION=$(CALICOCTL_GIT_REVISION) \ + -X $(PACKAGE_NAME)/v3/calicoctl/commands/common.VERSION=$(GIT_VERSION) -s -w" .PHONY: clean ## Clean enough that a new release build will be clean diff --git a/calicoctl/calicoctl.go b/calicoctl/calicoctl.go index a4a7cf86f..068deffa6 100644 --- a/calicoctl/calicoctl.go +++ b/calicoctl/calicoctl.go @@ -50,11 +50,11 @@ func main() { datastore Calico datastore management. Options: - -h --help Show this screen. - -l --log-level= Set the log level (one of panic, fatal, error, - warn, info, debug) [default: panic] - --context= The name of the kubeconfig context to use. - --allow-version-mismatch Allow client and cluster versions mismatch. + -h --help Show this screen. + -l --log-level= Set the log level (one of panic, fatal, error, + warn, info, debug) [default: panic] + --context= The name of the kubeconfig context to use. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The %s is used to manage Calico network and security @@ -101,18 +101,9 @@ Description: command := arguments[""].(string) args := append([]string{command}, arguments[""].([]string)...) - // A "datastore migrate import" command should skip version mismatch checking, - // since the datastore will not have the cluster version set - isImport := args[0] == "datastore" && args[1] == "migrate" && args[2] == "import" - - // Check for client/cluster version mismatch. If a mismatch occurs, check for - // --allow-version-mismatch arg to override/fail. - allowMismatch, ok := arguments["--allow-version-mismatch"].(bool) - if (!ok || !allowMismatch) && !isImport { - if err = commands.VersionMismatch(args); err != nil { - fmt.Fprintf(os.Stderr, "%s\n Use --allow-version-mismatch to override.", err) - os.Exit(1) - } + // Propagate the '--allow-version-mismatch' arg to override version mismatch checking. + if allowMismatch, _ := arguments["--allow-version-mismatch"].(bool); allowMismatch { + args = append(args, "--allow-version-mismatch") } var err error diff --git a/calicoctl/commands/apply.go b/calicoctl/commands/apply.go index c509cc2cc..fdf604e55 100644 --- a/calicoctl/commands/apply.go +++ b/calicoctl/commands/apply.go @@ -30,7 +30,7 @@ import ( func Apply(args []string) error { doc := constants.DatastoreIntro + `Usage: apply --filename= [--recursive] [--skip-empty] - [--config=] [--namespace=] [--context=] + [--config=] [--namespace=] [--context=] [--allow-version-mismatch] Examples: # Apply a policy using the data in policy.yaml. @@ -40,21 +40,22 @@ Examples: cat policy.json | apply -f - Options: - -h --help Show this screen. - -f --filename= Filename to use to apply the resource. If set to - "-" loads from stdin. If filename is a directory, this command is - invoked for each .json .yaml and .yml file within that directory, - terminating after the first failure. - -R --recursive Process the filename specified in -f or --filename recursively. - --skip-empty Do not error if any files or directory specified using -f or --filename contain no - data. - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] - -n --namespace= Namespace of the resource. - Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. - Uses the default namespace if not specified. - --context= The name of the kubeconfig context to use. + -h --help Show this screen. + -f --filename= Filename to use to apply the resource. If set to + "-" loads from stdin. If filename is a directory, this command is + invoked for each .json .yaml and .yml file within that directory, + terminating after the first failure. + -R --recursive Process the filename specified in -f or --filename recursively. + --skip-empty Do not error if any files or directory specified using -f or --filename contain no + data. + -c --config= Path to the file containing connection + configuration in YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + -n --namespace= Namespace of the resource. + Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. + Uses the default namespace if not specified. + --context= The name of the kubeconfig context to use. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The apply command is used to create or replace a set of resources by filename diff --git a/calicoctl/commands/common/resources.go b/calicoctl/commands/common/resources.go index 7c1cc0eee..caca7efd9 100644 --- a/calicoctl/commands/common/resources.go +++ b/calicoctl/commands/common/resources.go @@ -128,6 +128,11 @@ func ExecuteConfigCommand(args map[string]interface{}, action action) CommandRes log.Info("Executing config command") + err := CheckVersionMismatch(args["--config"], args["--allow-version-mismatch"]) + if err != nil { + return CommandResults{Err: err} + } + errorOnEmpty := !argutils.ArgBoolOrFalse(args, "--skip-empty") if filename := args["--filename"]; filename != nil { diff --git a/calicoctl/commands/common/version_mismatch.go b/calicoctl/commands/common/version_mismatch.go new file mode 100644 index 000000000..b3ffce048 --- /dev/null +++ b/calicoctl/commands/common/version_mismatch.go @@ -0,0 +1,82 @@ +// Copyright (c) 2021 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package common + +import ( + "context" + "errors" + "fmt" + "strings" + + log "github.com/sirupsen/logrus" + + "github.com/projectcalico/calicoctl/v3/calicoctl/commands/clientmgr" + cerrors "github.com/projectcalico/libcalico-go/lib/errors" + "github.com/projectcalico/libcalico-go/lib/options" +) + +var VERSION string + +func CheckVersionMismatch(configArg, allowMismatchArg interface{}) error { + if allowMismatch, _ := allowMismatchArg.(bool); allowMismatch { + log.Infof("Skip version mismatch checking due to '--allow-version-mismatch' argument") + + return nil + } + + cf, _ := configArg.(string) + + client, err := clientmgr.NewClient(cf) + if err != nil { + // If we can't connect to the cluster, skip the check. Either we're running a command that + // doesn't need API access, in which case the check doesn't need to be run, or we'll + // fail on the actual command. + log.Infof("Skip version mismatch checking due to not being able to connect to the cluster") + + return nil + } + + ctx := context.Background() + + ci, err := client.ClusterInformation().Get(ctx, "default", options.GetOptions{}) + if err != nil { + var notFound cerrors.ErrorResourceDoesNotExist + if errors.As(err, ¬Found) { + // ClusterInformation does not exist, so skip version check. + log.Infof("Skip version mismatch checking due to ClusterInformation not being present") + + return nil + } + return fmt.Errorf("Unable to get Cluster Information to verify version mismatch: %w\n Use --allow-version-mismatch to override.", err) + } + + clusterv := ci.Spec.CalicoVersion + if clusterv == "" { + // CalicoVersion field not specified in the cluster, so skip check. + log.Infof("Skip version mismatch checking due to CalicoVersion not being set") + + return nil + } + + clusterv = strings.Split(clusterv, "-")[0] + + clientv := strings.Split(VERSION, "-")[0] + + if clusterv != clientv { + return fmt.Errorf("Version mismatch.\nClient Version: %s\nCluster Version: %s\nUse --allow-version-mismatch to override.", VERSION, clusterv) + } + + return nil +} diff --git a/calicoctl/commands/convert.go b/calicoctl/commands/convert.go index 74b2de644..5949711ca 100644 --- a/calicoctl/commands/convert.go +++ b/calicoctl/commands/convert.go @@ -36,7 +36,7 @@ import ( func Convert(args []string) error { doc := constants.DatastoreIntro + `Usage: convert --filename= - [--output=] [--ignore-validation] + [--output=] [--ignore-validation] [--allow-version-mismatch] Examples: # Convert the contents of policy.yaml to v3 policy. @@ -51,7 +51,8 @@ Options: "-" loads from stdin. -o --output= Output format. One of: yaml or json. [Default: yaml] - --ignore-validation Skip validation on the converted manifest. + --ignore-validation Skip validation on the converted manifest. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: @@ -71,6 +72,8 @@ Description: return nil } + // Note: Intentionally not check version mismatch for this command + var rp common.ResourcePrinter output := parsedArgs["--output"].(string) // Only supported output formats are yaml (default) and json. diff --git a/calicoctl/commands/create.go b/calicoctl/commands/create.go index d30cfad87..6418c6d26 100644 --- a/calicoctl/commands/create.go +++ b/calicoctl/commands/create.go @@ -30,7 +30,7 @@ import ( func Create(args []string) error { doc := constants.DatastoreIntro + `Usage: create --filename= [--recursive] [--skip-empty] - [--skip-exists] [--config=] [--namespace=] [--context=] + [--skip-exists] [--config=] [--namespace=] [--context=] [--allow-version-mismatch] Examples: # Create a policy using the data in policy.yaml. @@ -40,23 +40,24 @@ Examples: cat policy.json | create -f - Options: - -h --help Show this screen. - -f --filename= Filename to use to create the resource. If set to - "-" loads from stdin. If filename is a directory, this command is - invoked for each .json .yaml and .yml file within that directory, - terminating after the first failure. - -R --recursive Process the filename specified in -f or --filename recursively. - --skip-empty Do not error if any files or directory specified using -f or --filename contain no - data. - --skip-exists Skip over and treat as successful any attempts to - create an entry that already exists. - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] - -n --namespace= Namespace of the resource. - Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. - Uses the default namespace if not specified. - --context= The name of the kubeconfig context to use. + -h --help Show this screen. + -f --filename= Filename to use to create the resource. If set to + "-" loads from stdin. If filename is a directory, this command is + invoked for each .json .yaml and .yml file within that directory, + terminating after the first failure. + -R --recursive Process the filename specified in -f or --filename recursively. + --skip-empty Do not error if any files or directory specified using -f or --filename contain no + data. + --skip-exists Skip over and treat as successful any attempts to + create an entry that already exists. + -c --config= Path to the file containing connection + configuration in YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + -n --namespace= Namespace of the resource. + Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. + Uses the default namespace if not specified. + --context= The name of the kubeconfig context to use. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The create command is used to create a set of resources by filename or stdin. diff --git a/calicoctl/commands/datastore/migrate/export.go b/calicoctl/commands/datastore/migrate/export.go index 01ec83f70..dbb708bee 100644 --- a/calicoctl/commands/datastore/migrate/export.go +++ b/calicoctl/commands/datastore/migrate/export.go @@ -26,12 +26,12 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + apiv3 "github.com/projectcalico/api/pkg/apis/projectcalico/v3" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/clientmgr" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/common" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/constants" "github.com/projectcalico/calicoctl/v3/calicoctl/util" "github.com/projectcalico/libcalico-go/lib/apiconfig" - apiv3 "github.com/projectcalico/api/pkg/apis/projectcalico/v3" libapiv3 "github.com/projectcalico/libcalico-go/lib/apis/v3" "github.com/projectcalico/libcalico-go/lib/backend/k8s/conversion" ) @@ -80,13 +80,14 @@ var namespacedResources map[string]struct{} = map[string]struct{}{ func Export(args []string) error { doc := `Usage: - datastore migrate export [--config=] + datastore migrate export [--config=] [--allow-version-mismatch] Options: - -h --help Show this screen. - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] + -h --help Show this screen. + -c --config= Path to the file containing connection + configuration in YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + --allow-version-mismatch Allow client and cluster versions mismatch. Description: Export the contents of the etcdv3 datastore. Resources will be exported @@ -127,6 +128,11 @@ Description: return nil } + err = common.CheckVersionMismatch(parsedArgs["--config"], parsedArgs["--allow-version-mismatch"]) + if err != nil { + return err + } + cf := parsedArgs["--config"].(string) // Get the backend client. client, err := clientmgr.NewClient(cf) diff --git a/calicoctl/commands/datastore/migrate/import.go b/calicoctl/commands/datastore/migrate/import.go index 917c0ee63..cae4b76c1 100644 --- a/calicoctl/commands/datastore/migrate/import.go +++ b/calicoctl/commands/datastore/migrate/import.go @@ -49,15 +49,16 @@ import ( func Import(args []string) error { doc := `Usage: - datastore migrate import --filename= [--config=] + datastore migrate import --filename= [--config=] [--allow-version-mismatch] Options: - -h --help Show this screen. - -f --filename= Filename to use to import resources. If set to - "-" loads from stdin. - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] + -h --help Show this screen. + -f --filename= Filename to use to import resources. If set to + "-" loads from stdin. + -c --config= Path to the file containing connection + configuration in YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + --allow-version-mismatch Allow client and cluster versions mismatch. Description: Import the contents of the etcdv3 datastore from the file created by the @@ -75,6 +76,8 @@ Description: return nil } + // Note: Intentionally not check version mismatch for this command + cf := parsedArgs["--config"].(string) cfg, err := clientmgr.LoadClientConfig(cf) if err != nil { diff --git a/calicoctl/commands/datastore/migrate/lock.go b/calicoctl/commands/datastore/migrate/lock.go index 347d0b262..d44d8d53e 100644 --- a/calicoctl/commands/datastore/migrate/lock.go +++ b/calicoctl/commands/datastore/migrate/lock.go @@ -22,6 +22,7 @@ import ( "github.com/docopt/docopt-go" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/clientmgr" + "github.com/projectcalico/calicoctl/v3/calicoctl/commands/common" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/constants" "github.com/projectcalico/calicoctl/v3/calicoctl/util" "github.com/projectcalico/libcalico-go/lib/clientv3" @@ -30,13 +31,14 @@ import ( func Lock(args []string) error { doc := `Usage: - datastore migrate lock [--config=] + datastore migrate lock [--config=] [--allow-version-mismatch] Options: - -h --help Show this screen. - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] + -h --help Show this screen. + -c --config= Path to the file containing connection + configuration in YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + --allow-version-mismatch Allow client and cluster versions mismatch. Description: Lock the datastore to prepare it for migration. This prevents any new @@ -55,6 +57,11 @@ Description: return nil } + err = common.CheckVersionMismatch(parsedArgs["--config"], parsedArgs["--allow-version-mismatch"]) + if err != nil { + return err + } + cf := parsedArgs["--config"].(string) client, err := clientmgr.NewClient(cf) if err != nil { diff --git a/calicoctl/commands/datastore/migrate/unlock.go b/calicoctl/commands/datastore/migrate/unlock.go index 28cc062bb..c913bc601 100644 --- a/calicoctl/commands/datastore/migrate/unlock.go +++ b/calicoctl/commands/datastore/migrate/unlock.go @@ -22,6 +22,7 @@ import ( "github.com/docopt/docopt-go" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/clientmgr" + "github.com/projectcalico/calicoctl/v3/calicoctl/commands/common" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/constants" "github.com/projectcalico/calicoctl/v3/calicoctl/util" "github.com/projectcalico/libcalico-go/lib/options" @@ -29,13 +30,14 @@ import ( func Unlock(args []string) error { doc := `Usage: - datastore migrate unlock [--config=] + datastore migrate unlock [--config=] [--allow-version-mismatch] Options: - -h --help Show this screen. - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] + -h --help Show this screen. + -c --config= Path to the file containing connection + configuration in YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + --allow-version-mismatch Allow client and cluster versions mismatch. Description: Unlock the datastore to complete migration. This once again allows @@ -53,6 +55,11 @@ Description: return nil } + err = common.CheckVersionMismatch(parsedArgs["--config"], parsedArgs["--allow-version-mismatch"]) + if err != nil { + return err + } + cf := parsedArgs["--config"].(string) client, err := clientmgr.NewClient(cf) if err != nil { diff --git a/calicoctl/commands/delete.go b/calicoctl/commands/delete.go index 146f1fc9b..54d718911 100644 --- a/calicoctl/commands/delete.go +++ b/calicoctl/commands/delete.go @@ -31,7 +31,7 @@ func Delete(args []string) error { doc := constants.DatastoreIntro + `Usage: delete ( ( [...]) | --filename= [--recursive] [--skip-empty] ) - [--skip-not-exists] [--config=] [--namespace=] [--context=] + [--skip-not-exists] [--config=] [--namespace=] [--context=] [--allow-version-mismatch] Examples: # Delete a policy using the type and name specified in policy.yaml. @@ -44,24 +44,25 @@ Examples: delete policy foo bar Options: - -h --help Show this screen. - -s --skip-not-exists Skip over and treat as successful, resources that - don't exist. - -f --filename= Filename to use to delete the resource. If set to - "-" loads from stdin. If filename is a directory, this command is - invoked for each .json .yaml and .yml file within that directory, - terminating after the first failure. - -R --recursive Process the filename specified in -f or --filename recursively. - --skip-empty Do not error if any files or directory specified using -f or --filename contain no - data. - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] - -n --namespace= Namespace of the resource. - Only applicable to NetworkPolicy and WorkloadEndpoint. - Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. - Uses the default namespace if not specified. - --context= The name of the kubeconfig context to use. + -h --help Show this screen. + -s --skip-not-exists Skip over and treat as successful, resources that + don't exist. + -f --filename= Filename to use to delete the resource. If set to + "-" loads from stdin. If filename is a directory, this command is + invoked for each .json .yaml and .yml file within that directory, + terminating after the first failure. + -R --recursive Process the filename specified in -f or --filename recursively. + --skip-empty Do not error if any files or directory specified using -f or --filename contain no + data. + -c --config= Path to the file containing connection + configuration in YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + -n --namespace= Namespace of the resource. + Only applicable to NetworkPolicy and WorkloadEndpoint. + Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. + Uses the default namespace if not specified. + --context= The name of the kubeconfig context to use. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The delete command is used to delete a set of resources by filename or stdin, diff --git a/calicoctl/commands/get.go b/calicoctl/commands/get.go index 491661ee7..38b1af4b8 100644 --- a/calicoctl/commands/get.go +++ b/calicoctl/commands/get.go @@ -33,7 +33,7 @@ func Get(args []string) error { doc := constants.DatastoreIntro + `Usage: get ( ( [...]) | --filename= [--recursive] [--skip-empty] ) - [--output=] [--config=] [--namespace=] [--all-namespaces] [--export] [--context=] + [--output=] [--config=] [--namespace=] [--all-namespaces] [--export] [--context=] [--allow-version-mismatch] Examples: # List all policy in default output format. @@ -61,10 +61,11 @@ Options: Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. Uses the default namespace if not specified. -A --all-namespaces If present, list the requested object(s) across all namespaces. - --export If present, returns the requested object(s) stripped of + --export If present, returns the requested object(s) stripped of cluster-specific information. This flag will be ignored if is not specified. - --context= The name of the kubeconfig context to use. + --context= The name of the kubeconfig context to use. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The get command is used to display a set of resources by filename or stdin, diff --git a/calicoctl/commands/ipam/check.go b/calicoctl/commands/ipam/check.go index cdba9662a..702fd0e4d 100644 --- a/calicoctl/commands/ipam/check.go +++ b/calicoctl/commands/ipam/check.go @@ -37,6 +37,7 @@ import ( bapi "github.com/projectcalico/libcalico-go/lib/backend/api" "github.com/projectcalico/libcalico-go/lib/options" + "github.com/projectcalico/calicoctl/v3/calicoctl/commands/common" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/constants" "github.com/projectcalico/calicoctl/v3/calicoctl/util" @@ -46,16 +47,17 @@ import ( // IPAM takes keyword with an IP address then calls the subcommands. func Check(args []string, version string) error { doc := constants.DatastoreIntro + `Usage: - ipam check [--config=] [--show-all-ips] [--show-problem-ips] [-o ] + ipam check [--config=] [--show-all-ips] [--show-problem-ips] [-o ] [--allow-version-mismatch] Options: - -h --help Show this screen. - -o --output= Path to output report file. - --show-all-ips Print all IPs that are checked. - --show-problem-ips Print all IPs that are leaked or not allocated properly. - -c --config= Path to the file containing connection configuration in - YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] + -h --help Show this screen. + -o --output= Path to output report file. + --show-all-ips Print all IPs that are checked. + --show-problem-ips Print all IPs that are leaked or not allocated properly. + -c --config= Path to the file containing connection configuration in + YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The ipam check command checks the integrity of the IPAM datastructures against Kubernetes. @@ -71,6 +73,12 @@ Description: if len(parsedArgs) == 0 { return nil } + + err = common.CheckVersionMismatch(parsedArgs["--config"], parsedArgs["--allow-version-mismatch"]) + if err != nil { + return err + } + ctx := context.Background() // Create a new backend client from env vars. diff --git a/calicoctl/commands/ipam/configure.go b/calicoctl/commands/ipam/configure.go index b655d0124..0b6a1ec55 100644 --- a/calicoctl/commands/ipam/configure.go +++ b/calicoctl/commands/ipam/configure.go @@ -23,6 +23,7 @@ import ( docopt "github.com/docopt/docopt-go" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/clientmgr" + "github.com/projectcalico/calicoctl/v3/calicoctl/commands/common" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/constants" "github.com/projectcalico/calicoctl/v3/calicoctl/util" "github.com/projectcalico/libcalico-go/lib/ipam" @@ -51,7 +52,7 @@ func updateIPAMStrictAffinity(ctx context.Context, ipamClient ipam.Interface, en // Configure IPAM. func Configure(args []string) error { doc := constants.DatastoreIntro + `Usage: - ipam configure --strictaffinity= [--config=] + ipam configure --strictaffinity= [--config=] [--allow-version-mismatch] Options: -h --help Show this screen. @@ -60,6 +61,7 @@ Options: -c --config= Path to the file containing connection configuration in YAML or JSON format. [default: ` + constants.DefaultConfigPath + `] + --allow-version-mismatch Allow client and cluster versions mismatch. Description: Modify configuration for Calico IP address management. @@ -76,6 +78,11 @@ Description: return nil } + err = common.CheckVersionMismatch(parsedArgs["--config"], parsedArgs["--allow-version-mismatch"]) + if err != nil { + return err + } + ctx := context.Background() // Create a new backend client from env vars. diff --git a/calicoctl/commands/ipam/release.go b/calicoctl/commands/ipam/release.go index abde205b5..581404a25 100644 --- a/calicoctl/commands/ipam/release.go +++ b/calicoctl/commands/ipam/release.go @@ -28,6 +28,7 @@ import ( "github.com/projectcalico/calicoctl/v3/calicoctl/commands/argutils" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/clientmgr" + "github.com/projectcalico/calicoctl/v3/calicoctl/commands/common" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/constants" "github.com/projectcalico/calicoctl/v3/calicoctl/util" client "github.com/projectcalico/libcalico-go/lib/clientv3" @@ -36,16 +37,17 @@ import ( // IPAM takes keyword with an IP address then calls the subcommands. func Release(args []string, version string) error { doc := constants.DatastoreIntro + `Usage: - ipam release [--ip=] [--from-report=] [--config=] [--force] + ipam release [--ip=] [--from-report=] [--config=] [--force] [--allow-version-mismatch] Options: - -h --help Show this screen. - --ip= IP address to release. - --from-report= Release all leaked addresses from the report. - --force Force release of leaked addresses. - -c --config= Path to the file containing connection configuration in - YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] + -h --help Show this screen. + --ip= IP address to release. + --from-report= Release all leaked addresses from the report. + --force Force release of leaked addresses. + -c --config= Path to the file containing connection configuration in + YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The ipam release command releases an IP address from the Calico IP Address @@ -68,6 +70,11 @@ Description: return nil } + err = common.CheckVersionMismatch(parsedArgs["--config"], parsedArgs["--allow-version-mismatch"]) + if err != nil { + return err + } + ctx := context.Background() // Load config. diff --git a/calicoctl/commands/ipam/show.go b/calicoctl/commands/ipam/show.go index 036667727..2f4efa570 100644 --- a/calicoctl/commands/ipam/show.go +++ b/calicoctl/commands/ipam/show.go @@ -24,6 +24,7 @@ import ( "github.com/olekukonko/tablewriter" + "github.com/projectcalico/calicoctl/v3/calicoctl/commands/common" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/constants" "github.com/projectcalico/calicoctl/v3/calicoctl/util" "github.com/projectcalico/libcalico-go/lib/backend/model" @@ -245,17 +246,18 @@ func showConfiguration(ctx context.Context, ipamClient ipam.Interface) error { // IPAM takes keyword with an IP address then calls the subcommands. func Show(args []string) error { doc := constants.DatastoreIntro + `Usage: - ipam show [--ip= | --show-blocks | --show-borrowed | --show-configuration] [--config=] + ipam show [--ip= | --show-blocks | --show-borrowed | --show-configuration] [--config=] [--allow-version-mismatch] Options: - -h --help Show this screen. - --ip= Report whether this specific IP address is in use. - --show-blocks Show detailed information for IP blocks as well as pools. - --show-borrowed Show detailed information for "borrowed" IP addresses. - --show-configuration Show current Calico IPAM configuration. - -c --config= Path to the file containing connection configuration in - YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] + -h --help Show this screen. + --ip= Report whether this specific IP address is in use. + --show-blocks Show detailed information for IP blocks as well as pools. + --show-borrowed Show detailed information for "borrowed" IP addresses. + --show-configuration Show current Calico IPAM configuration. + -c --config= Path to the file containing connection configuration in + YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The ipam show command prints information about a given IP address, or about @@ -272,6 +274,12 @@ Description: if len(parsedArgs) == 0 { return nil } + + err = common.CheckVersionMismatch(parsedArgs["--config"], parsedArgs["--allow-version-mismatch"]) + if err != nil { + return err + } + ctx := context.Background() // Create a new backend client from env vars. diff --git a/calicoctl/commands/label.go b/calicoctl/commands/label.go index b5d5c4c2d..4db8e90f2 100644 --- a/calicoctl/commands/label.go +++ b/calicoctl/commands/label.go @@ -34,7 +34,7 @@ func Label(args []string) error { label ( ( = [--overwrite] | --remove ) - [--config=] [--namespace=] [--context=]) + [--config=] [--namespace=] [--context=]) [--allow-version-mismatch] @@ -57,14 +57,15 @@ Options: -n --namespace= Namespace of the resource. Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. Uses the default namespace if not specified. - --overwrite If true, overwrite the value when the key is already + --overwrite If true, overwrite the value when the key is already present in labels. Otherwise reports error when the labeled resource already have the key in its labels. Can not be used with --remove. - --remove If true, remove the specified key in labels of the + --remove If true, remove the specified key in labels of the resource. Reports error when specified key does not exist. Can not be used with --overwrite. - --context= The name of the kubeconfig context to use. + --context= The name of the kubeconfig context to use. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The label command is used to add or update a label on a resource. Resource types diff --git a/calicoctl/commands/node/checksystem.go b/calicoctl/commands/node/checksystem.go index 957035bfb..5066a054e 100644 --- a/calicoctl/commands/node/checksystem.go +++ b/calicoctl/commands/node/checksystem.go @@ -64,7 +64,7 @@ var overrideBootFile = "" // Checksystem checks host system for compatible versions func Checksystem(args []string) error { doc := `Usage: - node checksystem [--kernel-config=] + node checksystem [--kernel-config=] [--allow-version-mismatch] Options: -h --help Show this screen. @@ -76,6 +76,7 @@ Options: "/usr/src/linux-kernelVersion/.config", "/usr/src/linux-headers-kernelVersion/.config", "/lib/modules/kernelVersion/build/.config" + --allow-version-mismatch Allow client and cluster versions mismatch. Description: Check the compatibility of this compute host to run a Calico node instance. @@ -91,6 +92,9 @@ Description: if len(parsedArgs) == 0 { return nil } + + // Note: Intentionally not check version mismatch for this command + if parsedArgs["--kernel-config"] != nil { overrideBootFile = parsedArgs["--kernel-config"].(string) } @@ -204,8 +208,8 @@ func checkKernelModules() error { printResult(v, "OK") } else if modulesBootPath != "" && checkModule(modulesBootPath, i, kernelVersionStr, "^%s=.") == nil { printResult(v, "OK") - // Since `xt_icmp` and `xt_icmp6` are not available in most distros anymore as a last resort - // this `if` condition will check currently loaded modules in iptables using `ip_tables_matches` file. + // Since `xt_icmp` and `xt_icmp6` are not available in most distros anymore as a last resort + // this `if` condition will check currently loaded modules in iptables using `ip_tables_matches` file. } else if checkModule(modulesLoadedIPtables, i, kernelVersionStr, "^%s$") == nil { printResult(v, "OK") } else { diff --git a/calicoctl/commands/node/diags.go b/calicoctl/commands/node/diags.go index 46db75a2e..14274a31e 100644 --- a/calicoctl/commands/node/diags.go +++ b/calicoctl/commands/node/diags.go @@ -43,12 +43,13 @@ type diagCmd struct { func Diags(args []string) error { var err error doc := `Usage: - node diags [--log-dir=] + node diags [--log-dir=] [--allow-version-mismatch] Options: - -h --help Show this screen. - --log-dir= The directory containing Calico logs. - [default: /var/log/calico] + -h --help Show this screen. + --log-dir= The directory containing Calico logs. + [default: /var/log/calico] + --allow-version-mismatch Allow client and cluster versions mismatch. Description: This command is used to gather diagnostic information from a Calico node. @@ -74,6 +75,8 @@ Description: return nil } + // Note: Intentionally not check version mismatch for this command + return runDiags(arguments["--log-dir"].(string)) } diff --git a/calicoctl/commands/node/run.go b/calicoctl/commands/node/run.go index bc3080608..6e069c8c2 100644 --- a/calicoctl/commands/node/run.go +++ b/calicoctl/commands/node/run.go @@ -69,6 +69,7 @@ func Run(args []string) error { [--no-default-ippools] [--dryrun] [--init-system] + [--allow-version-mismatch] Options: -h --help Show this screen. @@ -144,8 +145,10 @@ Options: configuration in YAML or JSON format. [default: ` + constants.DefaultConfigPath + `] --felix-config= - Path to the file containing Felix - configuration in YAML or JSON format. + Path to the file containing Felix + configuration in YAML or JSON format. + --allow-version-mismatch + Allow client and cluster versions mismatch. Description: This command is used to start a calico/node container instance which provides @@ -164,6 +167,8 @@ Description: return nil } + // Note: Intentionally not check version mismatch for this command + // Extract all the parameters. ipv4 := argutils.ArgStringOrBlank(arguments, "--ip") ipv6 := argutils.ArgStringOrBlank(arguments, "--ip6") diff --git a/calicoctl/commands/node/status.go b/calicoctl/commands/node/status.go index 95d683547..292e9bfbe 100644 --- a/calicoctl/commands/node/status.go +++ b/calicoctl/commands/node/status.go @@ -39,10 +39,11 @@ import ( // Status prints status of the node and returns error (if any) func Status(args []string) error { doc := `Usage: - node status + node status [--allow-version-mismatch] Options: - -h --help Show this screen. + -h --help Show this screen. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: Check the status of the Calico node instance. This includes the status and @@ -60,6 +61,8 @@ Description: return nil } + // Note: Intentionally not check version mismatch for this command + // Must run this command as root to be able to connect to BIRD sockets enforceRoot() diff --git a/calicoctl/commands/patch.go b/calicoctl/commands/patch.go index 72574769a..016770aba 100644 --- a/calicoctl/commands/patch.go +++ b/calicoctl/commands/patch.go @@ -29,7 +29,7 @@ import ( func Patch(args []string) error { doc := constants.DatastoreIntro + `Usage: - patch --patch= [--type=] [--config=] [--namespace=] [--context=] + patch --patch= [--type=] [--config=] [--namespace=] [--context=] [--allow-version-mismatch] Examples: # Partially update a node using a strategic merge patch. @@ -39,19 +39,20 @@ Examples: patch node node-0 --patch '{"spec":{"bgp": {"routeReflectorClusterID": "CLUSTER_ID"}}}' --type json Options: - -h --help Show this screen. - -p --patch= Spec to use to patch the resource. - -t --type= Format of patch type: - strategic Strategic merge patch (default) - json JSON Patch, RFC 6902 (not yet implemented) - merge JSON Merge Patch, RFC 7386 (not yet implemented) - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] - -n --namespace= Namespace of the resource. - Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. - Uses the default namespace if not specified. - --context= The name of the kubeconfig context to use. + -h --help Show this screen. + -p --patch= Spec to use to patch the resource. + -t --type= Format of patch type: + strategic Strategic merge patch (default) + json JSON Patch, RFC 6902 (not yet implemented) + merge JSON Merge Patch, RFC 7386 (not yet implemented) + -c --config= Path to the file containing connection + configuration in YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + -n --namespace= Namespace of the resource. + Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. + Uses the default namespace if not specified. + --context= The name of the kubeconfig context to use. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The patch command is used to patch a specific resource by type and identifiers in place. diff --git a/calicoctl/commands/replace.go b/calicoctl/commands/replace.go index 73762ccb4..3758cd448 100644 --- a/calicoctl/commands/replace.go +++ b/calicoctl/commands/replace.go @@ -32,7 +32,7 @@ import ( func Replace(args []string) error { doc := constants.DatastoreIntro + `Usage: replace --filename= [--recursive] [--skip-empty] - [--config=] [--namespace=] [--context=] + [--config=] [--namespace=] [--context=] [--allow-version-mismatch] Examples: # Replace a policy using the data in policy.yaml. @@ -42,21 +42,22 @@ Examples: cat policy.json | replace -f - Options: - -h --help Show this screen. - -f --filename= Filename to use to replace the resource. If set - to "-" loads from stdin. If filename is a directory, this command is - invoked for each .json .yaml and .yml file within that directory, - terminating after the first failure. - -R --recursive Process the filename specified in -f or --filename recursively. - --skip-empty Do not error if any files or directory specified using -f or --filename contain no - data. - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] - -n --namespace= Namespace of the resource. - Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. - Uses the default namespace if not specified. - --context= The name of the kubeconfig context to use. + -h --help Show this screen. + -f --filename= Filename to use to replace the resource. If set + to "-" loads from stdin. If filename is a directory, this command is + invoked for each .json .yaml and .yml file within that directory, + terminating after the first failure. + -R --recursive Process the filename specified in -f or --filename recursively. + --skip-empty Do not error if any files or directory specified using -f or --filename contain no + data. + -c --config= Path to the file containing connection + configuration in YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + -n --namespace= Namespace of the resource. + Only applicable to NetworkPolicy, NetworkSet, and WorkloadEndpoint. + Uses the default namespace if not specified. + --context= The name of the kubeconfig context to use. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: The replace command is used to replace a set of resources by filename or diff --git a/calicoctl/commands/version.go b/calicoctl/commands/version.go index bee1e3f0a..e22e5dddc 100644 --- a/calicoctl/commands/version.go +++ b/calicoctl/commands/version.go @@ -16,7 +16,6 @@ package commands import ( "context" - "errors" "fmt" "os" "strings" @@ -25,7 +24,6 @@ import ( "github.com/docopt/docopt-go" v3 "github.com/projectcalico/api/pkg/apis/projectcalico/v3" - cerrors "github.com/projectcalico/libcalico-go/lib/errors" "github.com/projectcalico/libcalico-go/lib/options" "github.com/projectcalico/calicoctl/v3/calicoctl/commands/argutils" @@ -44,15 +42,16 @@ func init() { func Version(args []string) error { doc := `Usage: - version [--config=] [--poll=] + version [--config=] [--poll=] [--allow-version-mismatch] Options: - -h --help Show this screen. - -c --config= Path to the file containing connection configuration in - YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] - --poll= Poll for changes to the cluster information at a frequency specified using POLL duration - (e.g. 1s, 10m, 2h etc.). A value of 0 (the default) disables polling. + -h --help Show this screen. + -c --config= Path to the file containing connection configuration in + YAML or JSON format. + [default: ` + constants.DefaultConfigPath + `] + --poll= Poll for changes to the cluster information at a frequency specified using POLL duration + (e.g. 1s, 10m, 2h etc.). A value of 0 (the default) disables polling. + --allow-version-mismatch Allow client and cluster versions mismatch. Description: Display the version of . @@ -69,6 +68,8 @@ Description: return nil } + // Note: Intentionally not check version mismatch for this command + // Parse the poll duration. var pollDuration time.Duration var ci *v3.ClusterInformation @@ -132,111 +133,3 @@ Description: return err } - -func VersionMismatch(args []string) error { - // We need to "look ahead" to see if config or context have been passed in the args - doc := `Usage: - [options] [...] - -Options: - -h --help Show this screen. - -c --config= Path to the file containing connection - configuration in YAML or JSON format. - [default: ` + constants.DefaultConfigPath + `] - --context= The name of the kubeconfig context to use. - -a - -A --all-namespaces - --as= - --backend=(bird|gobgp|none) - --dryrun - --export - --felix-config= - -f --filename= - --force - --from-report= - --ignore-validation - --init-system - --ip6-autodetection-method= - --ip6= - --ip-autodetection-method= - --ip= - --kernel-config= - --log-dir= - --name= - -n --namespace= - --no-default-ippools - --node-image= - -o --output= - --overwrite - --poll= - -p --patch= - --remove - -R --recursive - --show-all-ips - --show-blocks - --show-borrowed - --show-configuration - --show-problem-ips - --skip-empty - --skip-exists - -s --skip-not-exists - --strictaffinity= - -t --type= - -Description: - This is an intermediate parser for version mismatch verification and should - contain every command line option used everywhere in . If there - is an error at this point, there probably is some command line option that - should be added to this docstring. -` - // Replace all instances of BINARY_NAME with the name of the binary. - name, _ := util.NameAndDescription() - doc = strings.ReplaceAll(doc, "", name) - - parsedArgs, err := docopt.ParseArgs(doc, args, "") - if err != nil { - return fmt.Errorf("docopts parsing for version mismatch verification error: %w", err) - } - - if context := parsedArgs["--context"]; context != nil { - os.Setenv("K8S_CURRENT_CONTEXT", context.(string)) - } - - cf, _ := parsedArgs["--config"].(string) - - client, err := clientmgr.NewClient(cf) - if err != nil { - // If we can't connect to the cluster, skip the check. Either we're running a command that - // doesn't need API access, in which case the check doesn't need to be run, or we'll - // fail on the actual command. - return nil - } - - ctx := context.Background() - - ci, err := client.ClusterInformation().Get(ctx, "default", options.GetOptions{}) - if err != nil { - var notFound cerrors.ErrorResourceDoesNotExist - if errors.As(err, ¬Found) { - // ClusterInformation does not exist, so skip version check. - return nil - } - return fmt.Errorf("Unable to get Cluster Information to verify version mismatch: %w", err) - } - - clusterv := ci.Spec.CalicoVersion - if clusterv == "" { - // CalicoVersion field not specified in the cluster, so skip check. - return nil - } else { - clusterv = strings.Split(clusterv, "-")[0] - } - - clientv := strings.Split(VERSION, "-")[0] - - if clusterv != clientv { - return fmt.Errorf("Version mismatch.\nClient Version: %s\nCluster Version: %s", VERSION, clusterv) - } - - return nil -} diff --git a/tests/st/calicoctl/test_flags.py b/tests/st/calicoctl/test_flags.py index b86ba5761..f5779a83d 100644 --- a/tests/st/calicoctl/test_flags.py +++ b/tests/st/calicoctl/test_flags.py @@ -54,17 +54,22 @@ def test_version_mismatch(self): # Assert that the error is not "version mismatch" rc.assert_error("Invalid datastore type") - rc = calicoctl("version", allowVersionMismatch=False) + # CalicoVersion is unset in the cluster, expect no error + rc = calicoctl("replace", data=node_name1_rev1, allowVersionMismatch=False) rc.assert_no_error() + # Set the correct CalicoVersion in the cluster output = set_cluster_version() output.assert_no_error() - rc = calicoctl("version", allowVersionMismatch=False) - rc.assert_no_error() + # CalicoVersion is correct in the cluster, expect no error + rc = calicoctl("replace", data=node_name1_rev1, allowVersionMismatch=False) + output.assert_no_error() + # Set an incorrect CalicoVersion in the cluster output = set_cluster_version("v0.0.0.1.2.3") output.assert_no_error() - rc = calicoctl("version", allowVersionMismatch=False) + # CalicoVersion is incorrect in the cluster, expect error + rc = calicoctl("replace", data=node_name1_rev1, allowVersionMismatch=False) rc.assert_error("Version mismatch.") From c75e7d838499893897509c071d963a4ce6712c23 Mon Sep 17 00:00:00 2001 From: Pedro Coutinho Date: Tue, 28 Sep 2021 09:58:44 -0700 Subject: [PATCH 2/2] Fix version mismatch message formatting --- calicoctl/commands/common/version_mismatch.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/calicoctl/commands/common/version_mismatch.go b/calicoctl/commands/common/version_mismatch.go index b3ffce048..01d291877 100644 --- a/calicoctl/commands/common/version_mismatch.go +++ b/calicoctl/commands/common/version_mismatch.go @@ -59,7 +59,7 @@ func CheckVersionMismatch(configArg, allowMismatchArg interface{}) error { return nil } - return fmt.Errorf("Unable to get Cluster Information to verify version mismatch: %w\n Use --allow-version-mismatch to override.", err) + return fmt.Errorf("Unable to get Cluster Information to verify version mismatch: %w\nUse --allow-version-mismatch to override.\n", err) } clusterv := ci.Spec.CalicoVersion @@ -75,7 +75,7 @@ func CheckVersionMismatch(configArg, allowMismatchArg interface{}) error { clientv := strings.Split(VERSION, "-")[0] if clusterv != clientv { - return fmt.Errorf("Version mismatch.\nClient Version: %s\nCluster Version: %s\nUse --allow-version-mismatch to override.", VERSION, clusterv) + return fmt.Errorf("Version mismatch.\nClient Version: %s\nCluster Version: %s\nUse --allow-version-mismatch to override.\n", VERSION, clusterv) } return nil