From 1884055329fc192b39fac6d571138df42a22d1d1 Mon Sep 17 00:00:00 2001 From: Lars Lehtonen Date: Fri, 15 Sep 2017 18:35:29 -0700 Subject: [PATCH] cmd/kubeadm/app/util/apiclient: fix swallowed errors cmd/kubeadm/app/phases/upgrade: fix swallowed error cmd/kubeadm/app/phases/selfhosting: fix swallowed errors cmd/kubeadm/app/phases/certs: fix swallowed errors cmd/kubeadm/app/cmd: fix swallowed error cmd/kubeadm/app/cmd: descriptive error returns cmd/kubeadm/app/cmd: govet fixes cmd/kubeadm: error formatting --- cmd/kubeadm/app/cmd/init.go | 37 ++++++++++--------- cmd/kubeadm/app/phases/certs/certs_test.go | 9 +++++ .../phases/selfhosting/selfhosting_test.go | 6 +++ cmd/kubeadm/app/phases/upgrade/staticpods.go | 4 +- .../app/util/apiclient/clientbacked_dryrun.go | 6 +++ 5 files changed, 43 insertions(+), 19 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index 1816b273410..328360fc41c 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/cmd/kubeadm/app/cmd/init.go @@ -283,7 +283,7 @@ func (i *Init) Run(out io.Writer) error { realCertsDir := i.cfg.CertificatesDir certsDirToWriteTo, kubeConfigDir, manifestDir, err := getDirectoriesToUse(i.dryRun, i.cfg.CertificatesDir) if err != nil { - return err + return fmt.Errorf("error getting directories to use: %v", err) } // certsDirToWriteTo is gonna equal cfg.CertificatesDir in the normal case, but gonna be a temp directory if dryrunning i.cfg.CertificatesDir = certsDirToWriteTo @@ -312,12 +312,12 @@ func (i *Init) Run(out io.Writer) error { // PHASE 3: Bootstrap the control plane if err := controlplanephase.CreateInitStaticPodManifestFiles(manifestDir, i.cfg); err != nil { - return err + return fmt.Errorf("error creating init static pod manifest files: %v", err) } // Add etcd static pod spec only if external etcd is not configured if len(i.cfg.Etcd.Endpoints) == 0 { if err := etcdphase.CreateLocalEtcdStaticPodManifestFile(manifestDir, i.cfg); err != nil { - return err + return fmt.Errorf("error creating local etcd static pod manifest file: %v", err) } } @@ -326,13 +326,13 @@ func (i *Init) Run(out io.Writer) error { // If we're dry-running, print the generated manifests if err := printFilesIfDryRunning(i.dryRun, manifestDir); err != nil { - return err + return fmt.Errorf("error printing files on dryrun: %v", err) } // Create a kubernetes client and wait for the API server to be healthy (if not dryrunning) client, err := createClient(i.cfg, i.dryRun) if err != nil { - return err + return fmt.Errorf("error creating client: %v", err) } // waiter holds the apiclient.Waiter implementation of choice, responsible for querying the API server in various ways and waiting for conditions to be fulfilled @@ -355,12 +355,12 @@ func (i *Init) Run(out io.Writer) error { // Note: This is done right in the beginning of cluster initialization; as we might want to make other phases // depend on centralized information from this source in the future if err := uploadconfigphase.UploadConfiguration(i.cfg, client); err != nil { - return err + return fmt.Errorf("error uploading configuration: %v", err) } // PHASE 4: Mark the master with the right label/taint if err := markmasterphase.MarkMaster(client, i.cfg.NodeName); err != nil { - return err + return fmt.Errorf("error marking master: %v", err) } // PHASE 5: Set up the node bootstrap tokens @@ -371,15 +371,15 @@ func (i *Init) Run(out io.Writer) error { // Create the default node bootstrap token tokenDescription := "The default bootstrap token generated by 'kubeadm init'." if err := nodebootstraptokenphase.UpdateOrCreateToken(client, i.cfg.Token, false, i.cfg.TokenTTL.Duration, kubeadmconstants.DefaultTokenUsages, []string{kubeadmconstants.NodeBootstrapTokenAuthGroup}, tokenDescription); err != nil { - return err + return fmt.Errorf("error updating or creating token: %v", err) } // Create RBAC rules that makes the bootstrap tokens able to post CSRs if err := nodebootstraptokenphase.AllowBootstrapTokensToPostCSRs(client, k8sVersion); err != nil { - return err + return fmt.Errorf("error allowing bootstrap tokens to post CSRs: %v", err) } // Create RBAC rules that makes the bootstrap tokens able to get their CSRs approved automatically if err := nodebootstraptokenphase.AutoApproveNodeBootstrapTokens(client, k8sVersion); err != nil { - return err + return fmt.Errorf("error auto-approving node bootstrap tokens: %v", err) } // Create/update RBAC rules that makes the nodes to rotate certificates and get their CSRs approved automatically @@ -389,20 +389,18 @@ func (i *Init) Run(out io.Writer) error { // Create the cluster-info ConfigMap with the associated RBAC rules if err := clusterinfophase.CreateBootstrapConfigMapIfNotExists(client, adminKubeConfigPath); err != nil { - return err + return fmt.Errorf("error creating bootstrap configmap: %v", err) } if err := clusterinfophase.CreateClusterInfoRBACRules(client); err != nil { - return err + return fmt.Errorf("error creating clusterinfo RBAC rules: %v", err) } - // PHASE 6: Install and deploy all addons, and configure things as necessary - if err := dnsaddonphase.EnsureDNSAddon(i.cfg, client); err != nil { - return err + return fmt.Errorf("error ensuring dns addon: %v", err) } if err := proxyaddonphase.EnsureProxyAddon(i.cfg, client); err != nil { - return err + return fmt.Errorf("error ensuring proxy addon: %v", err) } // PHASE 7: Make the control plane self-hosted if feature gate is enabled @@ -411,7 +409,7 @@ func (i *Init) Run(out io.Writer) error { // plane components and remove the static manifests: fmt.Println("[self-hosted] Creating self-hosted control plane...") if err := selfhostingphase.CreateSelfHostedControlPlane(manifestDir, kubeConfigDir, i.cfg, client, waiter); err != nil { - return err + return fmt.Errorf("error creating self hosted control plane: %v", err) } } @@ -423,11 +421,14 @@ func (i *Init) Run(out io.Writer) error { // Load the CA certificate from so we can pin its public key caCert, err := pkiutil.TryLoadCertFromDisk(i.cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName) + if err != nil { + return fmt.Errorf("error loading ca cert from disk: %v", err) + } // Generate the Master host/port pair used by initDoneTempl masterHostPort, err := kubeadmutil.GetMasterHostPort(i.cfg) if err != nil { - return err + return fmt.Errorf("error getting master host port: %v", err) } ctx := map[string]string{ diff --git a/cmd/kubeadm/app/phases/certs/certs_test.go b/cmd/kubeadm/app/phases/certs/certs_test.go index a22f2e4ae77..b433e6ba422 100644 --- a/cmd/kubeadm/app/phases/certs/certs_test.go +++ b/cmd/kubeadm/app/phases/certs/certs_test.go @@ -323,6 +323,9 @@ func TestNewAPIServerCertAndKey(t *testing.T) { NodeName: "valid-hostname", } caCert, caKey, err := NewCACertAndKey() + if err != nil { + t.Fatalf("failed creation of ca cert and key: %v", err) + } apiServerCert, _, err := NewAPIServerCertAndKey(cfg, caCert, caKey) if err != nil { @@ -338,6 +341,9 @@ func TestNewAPIServerCertAndKey(t *testing.T) { func TestNewAPIServerKubeletClientCertAndKey(t *testing.T) { caCert, caKey, err := NewCACertAndKey() + if err != nil { + t.Fatalf("failed creation of ca cert and key: %v", err) + } apiClientCert, _, err := NewAPIServerKubeletClientCertAndKey(caCert, caKey) if err != nil { @@ -372,6 +378,9 @@ func TestNewFrontProxyCACertAndKey(t *testing.T) { func TestNewFrontProxyClientCertAndKey(t *testing.T) { frontProxyCACert, frontProxyCAKey, err := NewFrontProxyCACertAndKey() + if err != nil { + t.Fatalf("failed creation of ca cert and key: %v", err) + } frontProxyClientCert, _, err := NewFrontProxyClientCertAndKey(frontProxyCACert, frontProxyCAKey) if err != nil { diff --git a/cmd/kubeadm/app/phases/selfhosting/selfhosting_test.go b/cmd/kubeadm/app/phases/selfhosting/selfhosting_test.go index 40a5a0b07a2..237b07a4af8 100644 --- a/cmd/kubeadm/app/phases/selfhosting/selfhosting_test.go +++ b/cmd/kubeadm/app/phases/selfhosting/selfhosting_test.go @@ -474,6 +474,9 @@ func TestBuildDaemonSet(t *testing.T) { for _, rt := range tests { tempFile, err := createTempFileWithContent(rt.podBytes) + if err != nil { + t.Errorf("error creating tempfile with content:%v", err) + } defer os.Remove(tempFile) podSpec, err := loadPodSpecFromFile(tempFile) @@ -546,6 +549,9 @@ spec: for _, rt := range tests { tempFile, err := createTempFileWithContent([]byte(rt.content)) + if err != nil { + t.Errorf("error creating tempfile with content:%v", err) + } defer os.Remove(tempFile) _, err = loadPodSpecFromFile(tempFile) diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods.go b/cmd/kubeadm/app/phases/upgrade/staticpods.go index f368a66950d..bc04d8e8850 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods.go @@ -124,7 +124,9 @@ func StaticPodControlPlane(waiter apiclient.Waiter, pathMgr StaticPodPathManager // Write the updated static Pod manifests into the temporary directory fmt.Printf("[upgrade/staticpods] Writing upgraded Static Pod manifests to %q\n", pathMgr.TempManifestDir()) err = controlplane.CreateInitStaticPodManifestFiles(pathMgr.TempManifestDir(), cfg) - + if err != nil { + return fmt.Errorf("error creating init static pod manifest files: %v", err) + } for _, component := range constants.MasterComponents { // The old manifest is here; in the /etc/kubernetes/manifests/ currentManifestPath := pathMgr.RealManifestPath(component) diff --git a/cmd/kubeadm/app/util/apiclient/clientbacked_dryrun.go b/cmd/kubeadm/app/util/apiclient/clientbacked_dryrun.go index 2fbf95bdbb0..e531045fdf1 100644 --- a/cmd/kubeadm/app/util/apiclient/clientbacked_dryrun.go +++ b/cmd/kubeadm/app/util/apiclient/clientbacked_dryrun.go @@ -75,6 +75,9 @@ func (clg *ClientBackedDryRunGetter) HandleGetAction(action core.GetAction) (boo } unversionedObj, err := rc.Get(action.GetName(), metav1.GetOptions{}) + if err != nil { + return true, nil, err + } // If the unversioned object does not have .apiVersion; the inner object is probably nil if len(unversionedObj.GetAPIVersion()) == 0 { return true, nil, apierrors.NewNotFound(action.GetResource().GroupResource(), action.GetName()) @@ -100,6 +103,9 @@ func (clg *ClientBackedDryRunGetter) HandleListAction(action core.ListAction) (b } unversionedList, err := rc.List(listOpts) + if err != nil { + return true, nil, err + } // If the runtime.Object here is nil, we should return successfully with no result if unversionedList == nil { return true, unversionedList, nil