Skip to content

Commit

Permalink
updating error messages to be shown to end user (#6714)
Browse files Browse the repository at this point in the history
Signed-off-by: Durga Sarat Chandra Maddu <[email protected]>
  • Loading branch information
Dmaddu authored Feb 18, 2022
1 parent 7cb6070 commit ffdcca0
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 142 deletions.
22 changes: 7 additions & 15 deletions components/automate-cli/cmd/chef-automate/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ func runUpgradeCmd(cmd *cobra.Command, args []string) error {
if upgradeRunCmdFlags.version != "" && offlineMode {
return status.New(status.InvalidCommandArgsError, "--version and --airgap-bundle cannot be used together")
}
//Todo(milestone) check if upgradeRunCmdFlags.version is compatible with current version.

if offlineMode {
writer.Title("Installing airgap install bundle")
Expand Down Expand Up @@ -253,8 +252,11 @@ func statusUpgradeCmd(cmd *cobra.Command, args []string) error {
if resp.IsAirgapped {
writer.Printf("Automate is up-to-date with airgap bundle (%s)\n", resp.CurrentVersion)
} else if resp.CurrentVersion < resp.LatestAvailableVersion {
writer.Printf("Automate is out-of-date (current version: %s; latest available: %s; airgapped: %v)\n",
writer.Printf("Automate is out-of-date (current version: %s; next available version: %s; is Airgapped: %v)\n",
resp.CurrentVersion, resp.LatestAvailableVersion, resp.IsAirgapped)
if !resp.IsConvergeCompatable {
writer.Printf("Please manually run the major upgrade command to upgrade to %s\n", resp.LatestAvailableVersion)
}
} else {
writer.Printf("Automate is up-to-date (%s)\n", resp.CurrentVersion)
}
Expand All @@ -271,23 +273,13 @@ func statusUpgradeCmd(cmd *cobra.Command, args []string) error {
if resp.IsAirgapped {
writer.Titlef("Automate is upgrading to airgap bundle %s", resp.DesiredVersion)
} else {
if !resp.IsConvergeCompatable {
writer.Printf("Automate is out-of-date (current version: %s; latest available: %s; airgapped: %v)\n",
resp.CurrentVersion, resp.LatestAvailableVersion, resp.IsAirgapped)
} else {
writer.Titlef("Automate is upgrading to %s", resp.DesiredVersion)
}
writer.Titlef("Automate is upgrading to %s", resp.DesiredVersion)
}
} else {
if resp.IsAirgapped {
writer.Titlef("Automate is upgrading to airgap bundle %s", resp.LatestAvailableVersion)
} else {
if !resp.IsConvergeCompatable {
writer.Printf("Automate is out-of-date (current version: %s; latest available: %s; airgapped: %v)\n",
resp.CurrentVersion, resp.LatestAvailableVersion, resp.IsAirgapped)
} else {
writer.Titlef("Automate is upgrading to %s", resp.LatestAvailableVersion)
}
writer.Titlef("Automate is upgrading to %s", resp.LatestAvailableVersion)
}
}

Expand Down Expand Up @@ -371,7 +363,7 @@ func init() {
&upgradeRunCmdFlags.isMajorUpgrade,
"major",
false,
"will be used for major upgrade")
"This flag is only needed for major version upgrades")

upgradeCmd.AddCommand(upgradeRunCmd)
upgradeCmd.AddCommand(upgradeStatusCmd)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@ func (creator *InstallBundleCreator) Create(progress InstallBundleCreatorProgres
return "", err
}

//Todo(milestone): Add a check to see if the latest manifest is compatible with the current version
if creator.outputFile == "" {
creator.outputFile = fmt.Sprintf("automate-%s.aib", m.Version())
}
Expand Down
2 changes: 1 addition & 1 deletion components/automate-deployment/pkg/converge/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ func (phase *SelfUpgradePhase) Run(writer *eventWriter) error {
if err != nil {
return errors.Wrapf(err, "Could not parse deployment-service release %s", desiredReleaseStr)
}
//Todo(milestone) check if this code involves in upgrade process, if so modify the below comparison

if curRel < desRel {
// only upgrade if the release is newer. this is a safety net for
// the following situation: customers upgrading from a version of a2
Expand Down
4 changes: 0 additions & 4 deletions components/automate-deployment/pkg/manifest/client/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,7 @@ func (c *HTTP) manifestFromURL(ctx context.Context, url string) (*manifest.A2, e
return nil, err
}

//Todo(milestone) Append min compatible version needed to upgrade for the current manifest

m.HartOverrides = []habpkg.Hart{}

return m, nil
}

//Todo(milestone) method to find min compatible version needed to upgrade.
78 changes: 39 additions & 39 deletions components/automate-deployment/pkg/manifest/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,25 +172,25 @@ func TestGetCompatibleManifestVersion(t *testing.T) {
version: "20220113154113",
isMinorAvailable: false,
isMajorAvailable: true,
compatibleVersion: "1.2.1",
compatibleVersion: "3.2.1",
isError: false,
errorString: "",
},
{
input: "set1",
version: "1.0.0",
version: "3.0.0",
isMinorAvailable: true,
isMajorAvailable: false,
compatibleVersion: "1.2.1",
compatibleVersion: "3.2.1",
isError: false,
errorString: "",
},
{
input: "set1",
version: "1.2.1",
version: "3.2.1",
isMinorAvailable: false,
isMajorAvailable: true,
compatibleVersion: "2.1.2",
compatibleVersion: "4.1.2",
isError: false,
errorString: "",
},
Expand All @@ -199,25 +199,25 @@ func TestGetCompatibleManifestVersion(t *testing.T) {
version: "20220113154113",
isMinorAvailable: false,
isMajorAvailable: true,
compatibleVersion: "1.0.0",
compatibleVersion: "3.0.0",
isError: false,
errorString: "",
},
{
input: "set2",
version: "1.0.0",
version: "3.0.0",
isMinorAvailable: false,
isMajorAvailable: false,
compatibleVersion: "1.0.0",
compatibleVersion: "3.0.0",
isError: false,
errorString: "",
},
{
input: "set3",
version: "1.0.0",
version: "3.0.0",
isMinorAvailable: false,
isMajorAvailable: true,
compatibleVersion: "2.0.0",
compatibleVersion: "4.0.0",
isError: false,
errorString: "",
},
Expand All @@ -232,10 +232,10 @@ func TestGetCompatibleManifestVersion(t *testing.T) {
},
{
input: "set5",
version: "1.0.0",
version: "3.0.0",
isMinorAvailable: true,
isMajorAvailable: false,
compatibleVersion: "1.2.5",
compatibleVersion: "3.2.5",
isError: false,
errorString: "",
},
Expand Down Expand Up @@ -271,8 +271,8 @@ func TestGetMinCurrentVersion(t *testing.T) {
}{
{
input: "set1",
version: "2.1.2",
compatibleVersion: "1.2.1",
version: "4.1.2",
compatibleVersion: "3.2.1",
isError: false,
errorString: "",
},
Expand All @@ -285,21 +285,21 @@ func TestGetMinCurrentVersion(t *testing.T) {
},
{
input: "set1",
version: "3.4.5",
compatibleVersion: "2.1.2",
version: "5.4.5",
compatibleVersion: "4.1.2",
isError: false,
errorString: "",
},
{
input: "set1",
version: "1.0.0",
version: "3.0.0",
compatibleVersion: "20220113154113",
isError: false,
errorString: "",
},
{
input: "set1",
version: "1.2.1",
version: "3.2.1",
compatibleVersion: "20220113154113",
isError: false,
errorString: "",
Expand All @@ -313,15 +313,15 @@ func TestGetMinCurrentVersion(t *testing.T) {
},
{
input: "set2",
version: "1.0.0",
version: "3.0.0",
compatibleVersion: "20220113154113",
isError: false,
errorString: "",
},
{
input: "set3",
version: "2.0.0",
compatibleVersion: "1.0.0",
version: "4.0.0",
compatibleVersion: "3.0.0",
isError: false,
errorString: "",
},
Expand All @@ -334,8 +334,8 @@ func TestGetMinCurrentVersion(t *testing.T) {
},
{
input: "set5",
version: "1.2.5",
compatibleVersion: "1.0.0",
version: "3.2.5",
compatibleVersion: "3.0.0",
isError: false,
errorString: "",
},
Expand Down Expand Up @@ -371,10 +371,10 @@ func TestGetAllVersions(t *testing.T) {
} else if param[0] == "set2" {
resp = []string{
"20220113154113",
"22.0.0",
"22.2.6",
"23.2.4",
"22.2.16",
"3.0.0",
"3.2.6",
"4.2.4",
"3.2.16",
}
}

Expand All @@ -393,7 +393,7 @@ func TestGetAllVersions(t *testing.T) {
},
{
input: "set2",
output: []string{"20220113154113", "22.0.0", "22.2.6", "22.2.16", "23.2.4"},
output: []string{"20220113154113", "3.0.0", "3.2.6", "3.2.16", "4.2.4"},
},
}

Expand All @@ -418,23 +418,23 @@ func compatibleVerServer() *httptest.Server {
"20211220104140",
"20220113145751",
"20220113154113",
"1.0.0",
"1.1.9",
"1.2.1",
"2.0.0",
"2.1.2",
"3.4.5",
"3.0.0",
"3.1.9",
"3.2.1",
"4.0.0",
"4.1.2",
"5.4.5",
}
} else if param[0] == "set2" {
resp = []string{
"20220113154113",
"1.0.0",
"3.0.0",
}
} else if param[0] == "set3" {
resp = []string{
"20220113154113",
"1.0.0",
"2.0.0",
"3.0.0",
"4.0.0",
}
} else if param[0] == "set4" {
resp = []string{
Expand All @@ -447,8 +447,8 @@ func compatibleVerServer() *httptest.Server {
}
} else if param[0] == "set5" {
resp = []string{
"1.0.0",
"1.2.5",
"3.0.0",
"3.2.5",
}
}
bytes, _ := json.Marshal(resp)
Expand Down
18 changes: 9 additions & 9 deletions components/automate-deployment/pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ func (s *server) nextManifest() (*manifest.A2, error) {
return m, nil
}

logrus.Infof("Next upgradable manifest on channel %q (%s) is not compatible with current manifest (%s), ignoring",
logrus.Infof("The next available version %s in channel %q requires a manual upgrade with --major flag from the current version %s. Thus, ignoring auto-upgrade",
s.deployment.Channel(),
nextVersion,
s.deployment.CurrentReleaseManifest.Version())
Expand Down Expand Up @@ -1855,11 +1855,11 @@ func (s *server) IsValidUpgrade(ctx context.Context, req *api.UpgradeRequest) (*
if isMinorAvailable {
nextManifestVersion = compVersion
} else if isMajorAvailable {
return nil, status.Errorf(codes.InvalidArgument, "please use `chef-automate upgrade run --major` to further upgrade")
return nil, status.Errorf(codes.FailedPrecondition, "This is a Major upgrade. Please use `--major` flag in the above command")
}
} else { //major upgrade
if isMinorAvailable {
return nil, status.Errorf(codes.InvalidArgument, "minor/patch version is available, please use `chef-automate upgrade run`")
return nil, status.Errorf(codes.FailedPrecondition, "The next upgradable version is not a major version upgrade, please run the upgrade command without `--major` flag")
} else if isMajorAvailable {
nextManifestVersion = compVersion
}
Expand All @@ -1874,26 +1874,26 @@ func (s *server) IsValidUpgrade(ctx context.Context, req *api.UpgradeRequest) (*
//compare minimum compatible version with current version, i.e current version should be greater than or equal than min compatible version
minCompVersion := m.MinCompatibleVer
if minCompVersion == "" {
return nil, status.Error(codes.InvalidArgument, "mandatory minimum compatable version is missing")
return nil, status.Error(codes.FailedPrecondition, "The minimum compatible version field is missing in the manifest, please create a bundle with the latest automate-cli")
} else if !isCompatibleForAirgap(currentRelease, minCompVersion) {
return nil, status.Errorf(codes.OutOfRange, "the version specified %q is not compatible for the current version %q. The compatible version is %q", nextManifestVersion, currentRelease, minCompVersion)
return nil, status.Errorf(codes.FailedPrecondition, "The version specified %q is not compatible with the current version %q. Please first upgrade to the minimum compatible version %q", nextManifestVersion, currentRelease, minCompVersion)
}

//check for upgrade or degrade, we should not allow degrading
if isDegrade(currentRelease, nextManifestVersion) {
return nil, status.Errorf(codes.OutOfRange, "the version specified %q is not compatible for the current version %q", nextManifestVersion, currentRelease)
return nil, status.Errorf(codes.InvalidArgument, "The version specified %q is older than the current version %q", nextManifestVersion, currentRelease)
}

isActualMajorUpgrade := isMajorUpgrade(currentRelease, nextManifestVersion)

//check the upgrade is major or not, and if the upgrade is minor/patch, user should not provide --major flag
if !isActualMajorUpgrade && req.IsMajorUpgrade {
return nil, status.Errorf(codes.InvalidArgument, "please use `chef-automate upgrade run --airgap-bundle` to further upgrade")
return nil, status.Errorf(codes.FailedPrecondition, "The next upgradable version is not a major version upgrade, please run the upgrade command without `--major` flag")
}

//check the upgrade is major or not, and if the upgrade is major user should provide --major flag.
if isActualMajorUpgrade && !req.IsMajorUpgrade {
return nil, status.Errorf(codes.InvalidArgument, "please use `chef-automate upgrade run --major --airgap-bundle` to further upgrade")
return nil, status.Errorf(codes.FailedPrecondition, "This is a Major upgrade. Please use `--major` flag in the above command")
}

}
Expand All @@ -1904,7 +1904,7 @@ func (s *server) IsValidUpgrade(ctx context.Context, req *api.UpgradeRequest) (*
}

if nextManifestVersion != req.Version && !isCompatible(currentRelease, req.Version, nextManifestVersion) {
return nil, status.Errorf(codes.OutOfRange, "the version specified %q is not compatible for the current version %q", req.Version, currentRelease)
return nil, status.Errorf(codes.InvalidArgument, "The specified version %q is not compatible with the current version %q. The current version can be upgraded to %q", req.Version, currentRelease, nextManifestVersion)
}

nextManifestVersion = req.Version
Expand Down
Loading

0 comments on commit ffdcca0

Please sign in to comment.