From eb5d70adf7548c3d5bd0e493aba26307f33b5c6b Mon Sep 17 00:00:00 2001 From: shubham Date: Thu, 23 Jul 2020 17:13:45 +0530 Subject: [PATCH 1/2] fix(version): fix version checks of resources - this PR fixes the version checks f resources taking account of the custom version tags like 1.12.0-ce-RC1 - this PR also fixes the error overridden for utask upgrades Signed-off-by: shubham --- cmd/upgrade/executor/resource.go | 33 ++++++++++++++++---------------- pkg/upgrade/patch/cspc.go | 2 +- pkg/upgrade/patch/cspi.go | 6 ++++-- pkg/upgrade/patch/cv.go | 2 +- pkg/upgrade/patch/cvc.go | 2 +- pkg/upgrade/patch/cvr.go | 2 +- pkg/upgrade/patch/deployment.go | 5 +++-- pkg/upgrade/patch/service.go | 6 ++++-- 8 files changed, 32 insertions(+), 26 deletions(-) diff --git a/cmd/upgrade/executor/resource.go b/cmd/upgrade/executor/resource.go index 5908986b..f4e66d86 100644 --- a/cmd/upgrade/executor/resource.go +++ b/cmd/upgrade/executor/resource.go @@ -76,37 +76,38 @@ func NewUpgradeResourceJob() *cobra.Command { util.CheckErr(options.InitializeDefaults(cmd), util.Fatal) err := options.RunResourceUpgrade(cmd) if err != nil { - utaskObj, err := client.OpenebsV1alpha1().UpgradeTasks(openebsNamespace). + utaskObj, uerr := client.OpenebsV1alpha1().UpgradeTasks(openebsNamespace). Get(cr.Name, metav1.GetOptions{}) - if err != nil { - util.Fatal(err.Error()) + if uerr != nil { + util.Fatal(uerr.Error()) } - backoffLimit, err := getBackoffLimit(openebsNamespace) - if err != nil { - util.Fatal(err.Error()) + backoffLimit, uerr := getBackoffLimit(openebsNamespace) + if uerr != nil { + util.Fatal(uerr.Error()) } utaskObj.Status.Retries = utaskObj.Status.Retries + 1 if utaskObj.Status.Retries == backoffLimit { utaskObj.Status.Phase = v1Alpha1API.UpgradeError utaskObj.Status.CompletedTime = metav1.Now() } - _, err = client.OpenebsV1alpha1().UpgradeTasks(openebsNamespace). + _, uerr = client.OpenebsV1alpha1().UpgradeTasks(openebsNamespace). Update(utaskObj) - if err != nil { - util.Fatal(err.Error()) + if uerr != nil { + util.Fatal(uerr.Error()) } + util.Fatal(err.Error()) } else { - utaskObj, err := client.OpenebsV1alpha1().UpgradeTasks(openebsNamespace). + utaskObj, uerr := client.OpenebsV1alpha1().UpgradeTasks(openebsNamespace). Get(cr.Name, metav1.GetOptions{}) - if err != nil { - util.Fatal(err.Error()) + if uerr != nil { + util.Fatal(uerr.Error()) } utaskObj.Status.Phase = v1Alpha1API.UpgradeSuccess utaskObj.Status.CompletedTime = metav1.Now() - _, err = client.OpenebsV1alpha1().UpgradeTasks(openebsNamespace). + _, uerr = client.OpenebsV1alpha1().UpgradeTasks(openebsNamespace). Update(utaskObj) - if err != nil { - util.Fatal(err.Error()) + if uerr != nil { + util.Fatal(uerr.Error()) } } } @@ -167,7 +168,7 @@ func (u *UpgradeOptions) RunResourceUpgradeChecks(cmd *cobra.Command) error { // RunResourceUpgrade upgrades the given upgradeTask func (u *UpgradeOptions) RunResourceUpgrade(cmd *cobra.Command) error { if version.IsCurrentVersionValid(u.fromVersion) && version.IsDesiredVersionValid(u.toVersion) { - klog.Infof("Upgrading to %s", u.toVersion) + klog.Infof("Upgrading %s to %s", u.resourceKind, u.toVersion) err := upgrade.Exec(u.fromVersion, u.toVersion, u.resourceKind, u.name, diff --git a/pkg/upgrade/patch/cspc.go b/pkg/upgrade/patch/cspc.go index 37451587..6db95e23 100644 --- a/pkg/upgrade/patch/cspc.go +++ b/pkg/upgrade/patch/cspc.go @@ -59,7 +59,7 @@ func (c *CSPC) PreChecks(from, to string) error { return errors.Errorf("nil cspc object") } version := strings.Split(c.Object.VersionDetails.Status.Current, "-")[0] - if version != from && version != to { + if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cspc version %s is neither %s nor %s", version, diff --git a/pkg/upgrade/patch/cspi.go b/pkg/upgrade/patch/cspi.go index 6b13fe48..0d4f0f99 100644 --- a/pkg/upgrade/patch/cspi.go +++ b/pkg/upgrade/patch/cspi.go @@ -17,6 +17,8 @@ limitations under the License. package patch import ( + "strings" + apis "github.com/openebs/api/pkg/apis/cstor/v1" clientset "github.com/openebs/api/pkg/client/clientset/versioned" "github.com/pkg/errors" @@ -56,8 +58,8 @@ func (c *CSPI) PreChecks(from, to string) error { if c.Object == nil { return errors.Errorf("nil cspi object") } - version := c.Object.Labels["openebs.io/version"] - if version != from && version != to { + version := strings.Split(c.Object.Labels["openebs.io/version"], "-")[0] + if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cspi version %s is neither %s nor %s", version, diff --git a/pkg/upgrade/patch/cv.go b/pkg/upgrade/patch/cv.go index f9db333b..542c6df7 100644 --- a/pkg/upgrade/patch/cv.go +++ b/pkg/upgrade/patch/cv.go @@ -59,7 +59,7 @@ func (c *CV) PreChecks(from, to string) error { return errors.Errorf("nil cv object") } version := strings.Split(c.Object.VersionDetails.Status.Current, "-")[0] - if version != from && version != to { + if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cv version %s is neither %s nor %s", version, diff --git a/pkg/upgrade/patch/cvc.go b/pkg/upgrade/patch/cvc.go index ddbd9431..a2e7d05d 100644 --- a/pkg/upgrade/patch/cvc.go +++ b/pkg/upgrade/patch/cvc.go @@ -59,7 +59,7 @@ func (c *CVC) PreChecks(from, to string) error { return errors.Errorf("nil cvc object") } version := strings.Split(c.Object.VersionDetails.Status.Current, "-")[0] - if version != from && version != to { + if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cvc version %s is neither %s nor %s", version, diff --git a/pkg/upgrade/patch/cvr.go b/pkg/upgrade/patch/cvr.go index 17719c41..230f1e08 100644 --- a/pkg/upgrade/patch/cvr.go +++ b/pkg/upgrade/patch/cvr.go @@ -59,7 +59,7 @@ func (c *CVR) PreChecks(from, to string) error { return errors.Errorf("nil cvr object") } version := strings.Split(c.Object.VersionDetails.Status.Current, "-")[0] - if version != from && version != to { + if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cvr version %s is neither %s nor %s", version, diff --git a/pkg/upgrade/patch/deployment.go b/pkg/upgrade/patch/deployment.go index 1a2cc76e..01e4b82f 100644 --- a/pkg/upgrade/patch/deployment.go +++ b/pkg/upgrade/patch/deployment.go @@ -17,6 +17,7 @@ limitations under the License. package patch import ( + "strings" "time" deploy "github.com/openebs/maya/pkg/kubernetes/deployment/appsv1/v1alpha1" @@ -60,8 +61,8 @@ func (d *Deployment) PreChecks(from, to string) error { if d.Object == nil { return errors.Errorf("nil deployment object") } - version := d.Object.Labels["openebs.io/version"] - if version != from && version != to { + version := strings.Split(d.Object.Labels["openebs.io/version"], "-")[0] + if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "deployment version %s is neither %s nor %s", version, diff --git a/pkg/upgrade/patch/service.go b/pkg/upgrade/patch/service.go index babcee71..cdb3ce20 100644 --- a/pkg/upgrade/patch/service.go +++ b/pkg/upgrade/patch/service.go @@ -17,6 +17,8 @@ limitations under the License. package patch import ( + "strings" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -57,8 +59,8 @@ func (s *Service) PreChecks(from, to string) error { if name == "" { return errors.Errorf("missing service name") } - version := s.Object.Labels["openebs.io/version"] - if version != from && version != to { + version := strings.Split(s.Object.Labels["openebs.io/version"], "-")[0] + if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "service version %s is neither %s nor %s", version, From ed7fe706bddc03fa8c96eeb5a5d60d94fd65f0bc Mon Sep 17 00:00:00 2001 From: shubham Date: Tue, 28 Jul 2020 11:51:23 +0530 Subject: [PATCH 2/2] updated logs with appropriate values Signed-off-by: shubham --- cmd/upgrade/executor/resource.go | 2 +- pkg/upgrade/patch/cspc.go | 2 +- pkg/upgrade/patch/cspi.go | 2 +- pkg/upgrade/patch/cv.go | 2 +- pkg/upgrade/patch/cvc.go | 2 +- pkg/upgrade/patch/cvr.go | 2 +- pkg/upgrade/patch/deployment.go | 2 +- pkg/upgrade/patch/service.go | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/upgrade/executor/resource.go b/cmd/upgrade/executor/resource.go index f4e66d86..d181ee3a 100644 --- a/cmd/upgrade/executor/resource.go +++ b/cmd/upgrade/executor/resource.go @@ -168,7 +168,7 @@ func (u *UpgradeOptions) RunResourceUpgradeChecks(cmd *cobra.Command) error { // RunResourceUpgrade upgrades the given upgradeTask func (u *UpgradeOptions) RunResourceUpgrade(cmd *cobra.Command) error { if version.IsCurrentVersionValid(u.fromVersion) && version.IsDesiredVersionValid(u.toVersion) { - klog.Infof("Upgrading %s to %s", u.resourceKind, u.toVersion) + klog.Infof("Upgrading %s from %s to %s", u.resourceKind, u.fromVersion, u.toVersion) err := upgrade.Exec(u.fromVersion, u.toVersion, u.resourceKind, u.name, diff --git a/pkg/upgrade/patch/cspc.go b/pkg/upgrade/patch/cspc.go index 6db95e23..f5098db9 100644 --- a/pkg/upgrade/patch/cspc.go +++ b/pkg/upgrade/patch/cspc.go @@ -62,7 +62,7 @@ func (c *CSPC) PreChecks(from, to string) error { if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cspc version %s is neither %s nor %s", - version, + c.Object.VersionDetails.Status.Current, from, to, ) diff --git a/pkg/upgrade/patch/cspi.go b/pkg/upgrade/patch/cspi.go index 0d4f0f99..460e3ca3 100644 --- a/pkg/upgrade/patch/cspi.go +++ b/pkg/upgrade/patch/cspi.go @@ -62,7 +62,7 @@ func (c *CSPI) PreChecks(from, to string) error { if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cspi version %s is neither %s nor %s", - version, + c.Object.Labels["openebs.io/version"], from, to, ) diff --git a/pkg/upgrade/patch/cv.go b/pkg/upgrade/patch/cv.go index 542c6df7..6babde4c 100644 --- a/pkg/upgrade/patch/cv.go +++ b/pkg/upgrade/patch/cv.go @@ -62,7 +62,7 @@ func (c *CV) PreChecks(from, to string) error { if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cv version %s is neither %s nor %s", - version, + c.Object.VersionDetails.Status.Current, from, to, ) diff --git a/pkg/upgrade/patch/cvc.go b/pkg/upgrade/patch/cvc.go index a2e7d05d..ee929aac 100644 --- a/pkg/upgrade/patch/cvc.go +++ b/pkg/upgrade/patch/cvc.go @@ -62,7 +62,7 @@ func (c *CVC) PreChecks(from, to string) error { if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cvc version %s is neither %s nor %s", - version, + c.Object.VersionDetails.Status.Current, from, to, ) diff --git a/pkg/upgrade/patch/cvr.go b/pkg/upgrade/patch/cvr.go index 230f1e08..1f0dc31a 100644 --- a/pkg/upgrade/patch/cvr.go +++ b/pkg/upgrade/patch/cvr.go @@ -62,7 +62,7 @@ func (c *CVR) PreChecks(from, to string) error { if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "cvr version %s is neither %s nor %s", - version, + c.Object.VersionDetails.Status.Current, from, to, ) diff --git a/pkg/upgrade/patch/deployment.go b/pkg/upgrade/patch/deployment.go index 01e4b82f..4905eee7 100644 --- a/pkg/upgrade/patch/deployment.go +++ b/pkg/upgrade/patch/deployment.go @@ -65,7 +65,7 @@ func (d *Deployment) PreChecks(from, to string) error { if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "deployment version %s is neither %s nor %s", - version, + d.Object.Labels["openebs.io/version"], from, to, ) diff --git a/pkg/upgrade/patch/service.go b/pkg/upgrade/patch/service.go index cdb3ce20..1561a5a8 100644 --- a/pkg/upgrade/patch/service.go +++ b/pkg/upgrade/patch/service.go @@ -63,7 +63,7 @@ func (s *Service) PreChecks(from, to string) error { if version != strings.Split(from, "-")[0] && version != strings.Split(to, "-")[0] { return errors.Errorf( "service version %s is neither %s nor %s", - version, + s.Object.Labels["openebs.io/version"], from, to, )