Skip to content

Commit

Permalink
Refactoring for multierror
Browse files Browse the repository at this point in the history
  • Loading branch information
@paul-maidment
paul-maidment committed Sep 5, 2023
1 parent e846b14 commit eeca32d
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 50 deletions.
1 change: 1 addition & 0 deletions go.mod
View file
Expand Up @@ -3,6 +3,7 @@ module github.com/openshift/assisted-service
go 1.18

require (
github.com/hashicorp/go-multierror v1.1.1
github.com/IBM/netaddr v1.5.0
github.com/NYTimes/gziphandler v1.1.1
github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d
Expand Down
72 changes: 45 additions & 27 deletions pkg/localclusterimport/import_local_cluster.go
View file
Expand Up @@ -13,6 +13,7 @@ import (
v1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/hashicorp/go-multierror"
)

type LocalClusterImport struct {
Expand Down Expand Up @@ -121,6 +122,9 @@ func (i *LocalClusterImport) createAgentClusterInstall(numberOfControlPlaneNodes
}

func (i *LocalClusterImport) createClusterDeployment(pullSecret *v1.Secret, dns *configv1.DNS, kubeConfigSecret *v1.Secret, agentClusterInstall *hiveext.AgentClusterInstall) error {
if pullSecret == nil || dns == nil || kubeConfigSecret == nil || agentClusterInstall == nil {
return errors.New("invalid parameters")
}
// Create a cluster deployment in the local cluster namespace
clusterDeployment := &hivev1.ClusterDeployment{
Spec: hivev1.ClusterDeploymentSpec{
Expand Down Expand Up @@ -188,12 +192,14 @@ func (i *LocalClusterImport) shouldImportLocalCluster() (bool, error) {
return true, nil
}


func (i *LocalClusterImport) ImportLocalCluster() error {

var errorList error

clusterVersion, err := i.clusterImportOperations.GetClusterVersion("version")
if err != nil {
i.log.Errorf("unable to find cluster version info due to error: %s", err.Error())
return err
errorList = multierror.Append(nil, err)
}

shouldImportLocalCluster, err := i.shouldImportLocalCluster()
Expand All @@ -207,64 +213,76 @@ func (i *LocalClusterImport) ImportLocalCluster() error {
}

release_image := ""
if clusterVersion == nil || clusterVersion.Status.History[0].State != configv1.CompletedUpdate {
message := "cluster version info is empty, cannot proceed"
if clusterVersion != nil && clusterVersion.Status.History[0].State != configv1.CompletedUpdate {
message := "the release image in the cluster version is not ready yet, please wait for installation to complete and then restart the assisted-service"
i.log.Error(message)
return errors.New(message)
errorList = multierror.Append(errorList, errors.New(message))
}
release_image = clusterVersion.Status.History[0].Image

kubeConfigSecret, err := i.clusterImportOperations.GetSecret("openshift-kube-apiserver", "node-kubeconfigs")
if err != nil {
i.log.Errorf("unable to fetch local cluster kubeconfigs due to error %s", err.Error())
return err
errorList = multierror.Append(errorList, err)
}
i.log.Debugf("found secret %s in namespace %s", kubeConfigSecret.Name, kubeConfigSecret.Namespace)

pullSecret, err := i.clusterImportOperations.GetSecret("openshift-machine-api", "pull-secret")
if err != nil {
i.log.Errorf("unable to fetch pull secret due to error %s", err.Error())
return err
errorList = multierror.Append(errorList, err)
}
i.log.Debugf("found secret %s in namespace %s", pullSecret.Name, pullSecret.Namespace)


dns, err := i.clusterImportOperations.GetDNS()
if err != nil {
i.log.Errorf("could not fetch DNS due to error %s", err.Error())
return err
errorList = multierror.Append(errorList, err)
}

numberOfControlPlaneNodes, err := i.clusterImportOperations.GetNumberOfControlPlaneNodes()
if err != nil {
i.log.Errorf("unable to determine the number of control plane nodes due to error %s", err.Error())
return err
errorList = multierror.Append(errorList, err)
}

// If we already have errors before we start writing things, then let's stop
// better to let the user fix the problem than have to delete things later.
if errorList != nil {
return errorList
}
i.log.Debugf("number of control plane nodes is %d", numberOfControlPlaneNodes)

release_image = clusterVersion.Status.History[0].Image

err = i.createClusterImageSet(release_image)
if err != nil {
return err
errorList = multierror.Append(errorList, err)
}

err = i.createAdminKubeConfig(kubeConfigSecret)
if err != nil {
return err
if kubeConfigSecret != nil {
err = i.createAdminKubeConfig(kubeConfigSecret)
if err != nil {
errorList = multierror.Append(errorList, err)
}
}

err = i.createLocalClusterPullSecret(pullSecret)
if err != nil {
return err
if pullSecret != nil {
err = i.createLocalClusterPullSecret(pullSecret)
if err != nil {
errorList = multierror.Append(errorList, err)
}
}

agentClusterInstall, err := i.createAgentClusterInstall(numberOfControlPlaneNodes)
if err != nil {
return err
if numberOfControlPlaneNodes > 0 {
agentClusterInstall, err := i.createAgentClusterInstall(numberOfControlPlaneNodes)
if err != nil {
errorList = multierror.Append(errorList, err)
}
err = i.createClusterDeployment(pullSecret, dns, kubeConfigSecret, agentClusterInstall)
if err != nil {
errorList = multierror.Append(errorList, err)
}
}

err = i.createClusterDeployment(pullSecret, dns, kubeConfigSecret, agentClusterInstall)
if err != nil {
return err
if errorList != nil {
return errorList
}

i.log.Info("completed Day2 import of hub cluster")
Expand Down
84 changes: 61 additions & 23 deletions pkg/localclusterimport/import_local_cluster_test.go
View file
Expand Up @@ -7,6 +7,7 @@ import (

gomock "github.com/golang/mock/gomock"
"github.com/google/uuid"
"github.com/hashicorp/go-multierror"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
configv1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -349,111 +350,147 @@ var _ = Describe("ImportLocalCluster", func() {
Expect(localClusterImport.ImportLocalCluster()).To(BeNil())
})

It("should not proceed if cluster version does not exist", func() {
It("if no cluster version could be found then this should appear in multierrors", func() {
mockNoClusterVersionFound()
mockAgentClusterInstallNotPresent()
mockNodeKubeConfigsFound()
mockLocalClusterPullSecretFound()
mockClusterDNSFound()
mockNumberOfControlPlaneNodesFound()
result := localClusterImport.ImportLocalCluster()
apiStatus := result.(k8serrors.APIStatus)
multiErrors := result.(*multierror.Error)
apiStatus := multiErrors.Errors[0].(k8serrors.APIStatus)
Expect(apiStatus.Status().Reason).To(Equal(metav1.StatusReasonNotFound))
Expect(apiStatus.Status().Message).To(Equal(fmt.Sprintf("ClusterVersion.v1 \"%s\" not found", "version")))
})

It("should not proceed if node-kubeconfigs for the local cluster are not present", func() {
It("if node-kubeconfigs for the local cluster are not present then this should appear in multierrors", func() {
mockClusterVersionFound()
mockAgentClusterInstallNotPresent()
mockNodeKubeConfigsNotFound()
mockLocalClusterPullSecretFound()
mockClusterDNSFound()
mockNumberOfControlPlaneNodesFound()
result := localClusterImport.ImportLocalCluster()
apiStatus := result.(k8serrors.APIStatus)
multiErrors := result.(*multierror.Error)
apiStatus := multiErrors.Errors[0].(k8serrors.APIStatus)
Expect(apiStatus.Status().Reason).To(Equal(metav1.StatusReasonNotFound))
Expect(apiStatus.Status().Message).To(Equal(fmt.Sprintf("Secret.v1 \"%s\" not found", "node-kubeconfigs")))
})

It("should not proceed if pull-secret for the local cluster is not present", func() {
It("if pull-secret for the local cluster is not present then this should appear in multierrors", func() {
mockClusterVersionFound()
mockAgentClusterInstallNotPresent()
mockNodeKubeConfigsFound()
mockLocalClusterPullSecretNotFound()
mockClusterDNSFound()
mockNumberOfControlPlaneNodesFound()
result := localClusterImport.ImportLocalCluster()
apiStatus := result.(k8serrors.APIStatus)
multiErrors := result.(*multierror.Error)
apiStatus := multiErrors.Errors[0].(k8serrors.APIStatus)
Expect(apiStatus.Status().Reason).To(Equal(metav1.StatusReasonNotFound))
Expect(apiStatus.Status().Message).To(Equal(fmt.Sprintf("Secret.v1 \"%s\" not found", "pull-secret")))
})

It("should not proceed if dns for the local cluster is not present", func() {
It("if dns for the local cluster is not present then this should appear in multierrors", func() {
mockClusterVersionFound()
mockAgentClusterInstallNotPresent()
mockNodeKubeConfigsFound()
mockLocalClusterPullSecretFound()
mockClusterDNSNotFound()
mockNumberOfControlPlaneNodesFound()
result := localClusterImport.ImportLocalCluster()
apiStatus := result.(k8serrors.APIStatus)
multiErrors := result.(*multierror.Error)
apiStatus := multiErrors.Errors[0].(k8serrors.APIStatus)
Expect(apiStatus.Status().Reason).To(Equal(metav1.StatusReasonNotFound))
Expect(apiStatus.Status().Message).To(Equal(fmt.Sprintf("DNS.config.openshift.io/v1 \"%s\" not found", "cluster")))
})

It("should not proceed if unable to create image set", func() {
It("if unable to create image set, this shoud appear in multierrors", func() {
mockClusterVersionFound()
mockAgentClusterInstallNotPresent()
aciNotPresent := mockAgentClusterInstallNotPresent()
mockNodeKubeConfigsFound()
mockLocalClusterPullSecretFound()
mockClusterDNSFound()
mockNumberOfControlPlaneNodesFound()
mockFailedToCreateClusterImageSet()
mockCreateLocalClusterAdminKubeConfig()
mockCreateLocalClusterPullSecret()
mockCreateLocalClusterDeployment()
mockCreateAgentClusterInstall()
createdACIMock := mockAgentClusterInstallPresent()
gomock.InOrder(aciNotPresent, createdACIMock)
result := localClusterImport.ImportLocalCluster()
apiStatus := result.(k8serrors.APIStatus)
multiErrors := result.(*multierror.Error)
apiStatus := multiErrors.Errors[0].(k8serrors.APIStatus)
Expect(apiStatus.Status().Reason).To(Equal(metav1.StatusReasonBadRequest))
Expect(apiStatus.Status().Message).To(Equal("Invalid parameters"))
})

It("should not proceed if unable to create local cluster admin kubeconfig", func() {
It("if unable to create local cluster admin kubeconfig, this should appear in mutierrors", func() {
mockClusterVersionFound()
mockAgentClusterInstallNotPresent()
aciNotPresent := mockAgentClusterInstallNotPresent()
mockNodeKubeConfigsFound()
mockLocalClusterPullSecretFound()
mockClusterDNSFound()
mockNumberOfControlPlaneNodesFound()
mockCreateClusterImageSet()
mockFailedToCreateLocalClusterAdminKubeConfig()
mockCreateLocalClusterPullSecret()
mockCreateLocalClusterDeployment()
mockCreateAgentClusterInstall()
createdACIMock := mockAgentClusterInstallPresent()
gomock.InOrder(aciNotPresent, createdACIMock)
result := localClusterImport.ImportLocalCluster()
apiStatus := result.(k8serrors.APIStatus)
multiErrors := result.(*multierror.Error)
apiStatus := multiErrors.Errors[0].(k8serrors.APIStatus)
Expect(apiStatus.Status().Reason).To(Equal(metav1.StatusReasonBadRequest))
Expect(apiStatus.Status().Message).To(Equal("Invalid parameters"))
})

It("should not proceed if unable to create local cluster pull secret", func() {
It("if unable to create local cluster pull secret, this should appear in mutierrors", func() {
mockClusterVersionFound()
mockAgentClusterInstallNotPresent()
aciNotPresent := mockAgentClusterInstallNotPresent()
mockNodeKubeConfigsFound()
mockLocalClusterPullSecretFound()
mockClusterDNSFound()
mockNumberOfControlPlaneNodesFound()
mockCreateClusterImageSet()
mockCreateLocalClusterAdminKubeConfig()
mockFailedToCreateLocalClusterPullSecret()
mockCreateLocalClusterDeployment()
mockCreateAgentClusterInstall()
createdACIMock := mockAgentClusterInstallPresent()
gomock.InOrder(aciNotPresent, createdACIMock)
result := localClusterImport.ImportLocalCluster()
apiStatus := result.(k8serrors.APIStatus)
multiErrors := result.(*multierror.Error)
apiStatus := multiErrors.Errors[0].(k8serrors.APIStatus)
Expect(apiStatus.Status().Reason).To(Equal(metav1.StatusReasonBadRequest))
Expect(apiStatus.Status().Message).To(Equal("Invalid parameters"))
})

It("should not proceed if failed to create agent cluster install", func() {
It("if failed to create agent cluster install, this should appear in mutierrors", func() {
mockClusterVersionFound()
mockAgentClusterInstallNotPresent()
aciNotPresent := mockAgentClusterInstallNotPresent()
mockNodeKubeConfigsFound()
mockLocalClusterPullSecretFound()
mockClusterDNSFound()
mockNumberOfControlPlaneNodesFound()
mockCreateClusterImageSet()
mockCreateLocalClusterAdminKubeConfig()
mockCreateLocalClusterPullSecret()
mockCreateLocalClusterDeployment()
mockFailedToCreateAgentClusterInstall()
createdACIMock := mockAgentClusterInstallPresent()
gomock.InOrder(aciNotPresent, createdACIMock)
result := localClusterImport.ImportLocalCluster()
apiStatus := result.(k8serrors.APIStatus)
multiErrors := result.(*multierror.Error)
apiStatus := multiErrors.Errors[0].(k8serrors.APIStatus)
Expect(apiStatus.Status().Reason).To(Equal(metav1.StatusReasonBadRequest))
Expect(apiStatus.Status().Message).To(Equal("Invalid parameters"))
})

It("should give appropriate error if unable to create cluster deployment", func() {
It("if unable to create cluster deployment, this should appear in mutierrors", func() {
mockClusterVersionFound()
aciNotPresent := mockAgentClusterInstallNotPresent()
mockNodeKubeConfigsFound()
Expand All @@ -463,12 +500,13 @@ var _ = Describe("ImportLocalCluster", func() {
mockCreateClusterImageSet()
mockCreateLocalClusterAdminKubeConfig()
mockCreateLocalClusterPullSecret()
mockFailedToCreateLocalClusterDeployment()
mockCreateAgentClusterInstall()
createdACIMock := mockAgentClusterInstallPresent()
gomock.InOrder(aciNotPresent, createdACIMock)
mockFailedToCreateLocalClusterDeployment()
result := localClusterImport.ImportLocalCluster()
apiStatus := result.(k8serrors.APIStatus)
multiErrors := result.(*multierror.Error)
apiStatus := multiErrors.Errors[0].(k8serrors.APIStatus)
Expect(apiStatus.Status().Reason).To(Equal(metav1.StatusReasonBadRequest))
Expect(apiStatus.Status().Message).To(Equal("Invalid parameters"))
})
Expand Down

0 comments on commit eeca32d

Please sign in to comment.