fix: improve error messages for unsupported features

Error messages for supported features were intended to be
user-friendly, but they are still too abstract to easily understand.
They refer to the concept of "features", which is an internal Juju
construct and as such, should not be mentioned to users. This commit
attempts to improve the error messaging to make it more
straightforward.

apiserver/facades/client/application/application_unit_test.go

@@ -792,7 +792,9 @@ func (s *ApplicationSuite) TestSetCharmAssumesNotSatisfied(c *gc.C) {
 		ConfigSettings:  map[string]string{"stringOption": "value"},
 		CharmOrigin:     createCharmOriginFromURL(curl),
 	})
-	c.Assert(err, gc.ErrorMatches, `(?m).*Charm feature requirements cannot be met.*`)
+	c.Assert(err, gc.ErrorMatches, `(?s)Charm cannot be deployed because:
+  - charm requires feature "popcorn" but model does not support it
+`)
 }
 
 func (s *ApplicationSuite) TestSetCharmAssumesNotSatisfiedWithForce(c *gc.C) {

apiserver/facades/client/application/deploy_test.go

@@ -518,7 +518,8 @@ func (s *DeployLocalSuite) TestDeployWithUnmetCharmRequirements(c *gc.C) {
 			NumUnits:        1,
 			CharmOrigin:     corecharm.Origin{Platform: corecharm.Platform{OS: "ubuntu", Channel: "22.04"}},
 		})
-	c.Assert(err, gc.ErrorMatches, "(?m).*Charm feature requirements cannot be met.*")
+	c.Assert(err, gc.ErrorMatches, `(?s)Charm cannot be deployed because:
+  - charm requires Juju version >= 42.0.0.*`)
 }
 
 func (s *DeployLocalSuite) TestDeployWithUnmetCharmRequirementsAndForce(c *gc.C) {

caas/kubernetes/provider/k8s.go

@@ -2615,13 +2615,7 @@ func (k *kubernetesClient) SupportedFeatures() (assumes.FeatureSet, error) {
 		return fs, errors.Annotatef(err, "querying kubernetes API version")
 	}
 
-	fs.Add(
-		assumes.Feature{
-			Name:        "k8s-api",
-			Description: assumes.UserFriendlyFeatureDescriptions["k8s-api"],
-			Version:     k8sAPIVersion,
-		},
-	)
+	fs.Add(assumes.K8sAPIFeature(*k8sAPIVersion))
 	return fs, nil
 }
 

core/assumes/error.go

@@ -62,15 +62,6 @@ func requirementsNotSatisfied(message string, errList []error) *RequirementsNotS
 		}
 	}
 
-	if len(featNames) != 0 {
-		buf.WriteString("\nFeature descriptions:\n")
-		for _, featName := range featNames.SortedValues() {
-			buf.WriteString(fmt.Sprintf("  - %q: %s\n", featName, notSatFeatureDescrs[featName]))
-		}
-	}
-
-	buf.WriteString("\nFor additional information please see: " + featureDocsURL)
-
 	return &RequirementsNotSatisfiedError{
 		message: buf.String(),
 	}

core/assumes/error_test.go

@@ -0,0 +1,50 @@
+// Copyright 2024 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package assumes
+
+import (
+	"fmt"
+
+	"github.com/juju/version/v2"
+	gc "gopkg.in/check.v1"
+)
+
+type errorSuite struct{}
+
+var _ = gc.Suite(&errorSuite{})
+
+var errorTests = []struct {
+	description string
+	featureSet  FeatureSet
+	assumes     string
+	expectedErr string
+}{{
+	description: "Unsupported Juju version",
+	featureSet: FeatureSet{features: map[string]Feature{
+		"juju": JujuFeature(version.MustParse("2.9.42")),
+	}},
+	assumes:     "assumes:\n- juju >= 3.1",
+	expectedErr: "(?s).*charm requires Juju version >= 3.1.0.*",
+}, {
+	description: "Deploying k8s charm on machine cloud",
+	featureSet:  FeatureSet{},
+	assumes:     "assumes:\n- k8s-api",
+	expectedErr: "(?s).*charm must be deployed on a Kubernetes cloud.*",
+}, {
+	description: "k8s version too low",
+	featureSet: FeatureSet{features: map[string]Feature{
+		"k8s-api": K8sAPIFeature(version.MustParse("1.25.0")),
+	}},
+	assumes:     "assumes:\n- k8s-api >= 1.30",
+	expectedErr: "(?s).*charm requires Kubernetes version >= 1.30.*",
+}}
+
+func (*errorSuite) TestErrorMessages(c *gc.C) {
+	for _, test := range errorTests {
+		fmt.Println(test.description)
+		assumesTree := mustParseAssumesExpr(c, test.assumes)
+		err := test.featureSet.Satisfies(assumesTree)
+		c.Check(err, gc.ErrorMatches, test.expectedErr)
+	}
+}

core/assumes/features.go

@@ -0,0 +1,83 @@
+// Copyright 2024 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package assumes
+
+import (
+	"fmt"
+
+	"github.com/juju/version/v2"
+)
+
+var (
+	// A set of user-friendly descriptions for potentially supported
+	// features that are known to the controller. This allows us to
+	// generate better error messages when an "assumes" expression requests a
+	// feature that is not included in the feature set supported by the
+	// current model.
+	UserFriendlyFeatureDescriptions = map[string]string{
+		"juju":    "the version of Juju used by the model",
+		"k8s-api": "the Kubernetes API lets charms query and manipulate the state of API objects in a Kubernetes cluster",
+	}
+)
+
+// featureMissingErrs is a list of user-friendly error messages to return when
+// a given feature is expected by a charm, but not present in the model.
+var featureMissingErrs = map[string]string{
+	"juju":    "charm requires Juju", // this should never happen
+	"k8s-api": "charm must be deployed on a Kubernetes cloud",
+}
+
+// featureMissingErr returns a user-friendly error message to return when a
+// given feature is expected by a charm, but not present in the model.
+func featureMissingErr(featureName string) string {
+	if errStr, ok := featureMissingErrs[featureName]; ok {
+		return errStr
+	}
+	// We don't have a specific error message defined, so return a default.
+	return fmt.Sprintf("charm requires feature %q but model does not support it", featureName)
+}
+
+// featureVersionMismatchErrs is a list of functions which generate
+// user-friendly error messages when a given feature is present in the model,
+// but the version is lower than required by the charm.
+var featureVersionMismatchErrs = map[string]func(constraint, requiredVersion, actualVersion string) string{
+	"juju": func(c, rv, av string) string {
+		return fmt.Sprintf("charm requires Juju version %s %s, model has version %s", c, rv, av)
+	},
+	"k8s-api": func(c, rv, av string) string {
+		return fmt.Sprintf("charm requires Kubernetes version %s %s, model is running on version %s", c, rv, av)
+	},
+}
+
+// featureVersionMismatchErr returns a user-friendly error message to return when
+// a given feature is present in the model, but the version is lower than
+// required by the charm.
+func featureVersionMismatchErr(featureName, constraint, requiredVersion, actualVersion string) string {
+	if f, ok := featureVersionMismatchErrs[featureName]; ok {
+		return f(constraint, requiredVersion, actualVersion)
+	}
+	// We don't have a specific error message defined, so return a default.
+	return fmt.Sprintf("charm requires feature %q (version %s %s) but model currently supports version %s",
+		featureName, constraint, requiredVersion, actualVersion)
+}
+
+// JujuFeature returns a new Feature representing the Juju API for the given
+// version.
+func JujuFeature(ver version.Number) Feature {
+	return Feature{
+		Name:        "juju",
+		Description: UserFriendlyFeatureDescriptions["juju"],
+		Version:     &ver,
+	}
+}
+
+// K8sAPIFeature returns a new Feature representing the Kubernetes API for the
+// given version.
+func K8sAPIFeature(ver version.Number) Feature {
+	return Feature{
+		Name:        "k8s-api",
+		Description: UserFriendlyFeatureDescriptions["k8s-api"],
+		Version:     &ver,
+	}
+}

core/assumes/sat_checker.go

@@ -8,10 +8,6 @@ import (
 	"github.com/juju/errors"
 )
 
-// A link to a web page with additional information about features,
-// the Juju versions that support them etc.
-const featureDocsURL = "https://juju.is/docs/olm/supported-features"
-
 // satisfyExpr checks whether the feature set contents satisfy the provided
 // "assumes" expression. The function can process either feature or composite
 // expressions.
@@ -37,11 +33,9 @@ func satisfyExpr(fs FeatureSet, expr chassumes.Expression, exprTreeDepth int) er
 func satisfyFeatureExpr(fs FeatureSet, expr chassumes.FeatureExpression) error {
 	supported, defined := fs.Get(expr.Name)
 	if !defined {
+		errStr := featureMissingErr(expr.Name)
 		featDescr := UserFriendlyFeatureDescriptions[expr.Name]
-		return featureError(
-			expr.Name, featDescr,
-			"charm requires feature %q but model does not support it", expr.Name,
-		)
+		return featureError(expr.Name, featDescr, errStr)
 	}
 
 	// If the "assumes" feature expression does not specify a version or the
@@ -64,17 +58,16 @@ func satisfyFeatureExpr(fs FeatureSet, expr chassumes.FeatureExpression) error {
 		return nil
 	}
 
+	// Version mismatch. Get the nice error message.
+	errStr := featureVersionMismatchErr(expr.Name, string(expr.Constraint), expr.Version.String(), supported.Version.String())
+
 	var featDescr = supported.Description
 	if featDescr == "" {
 		// The feature set should always have a feature description.
 		// Try the fallback descriptions if it is missing.
 		featDescr = UserFriendlyFeatureDescriptions[featDescr]
 	}
-	return featureError(
-		expr.Name, featDescr,
-		"charm requires feature %q (version %s %s) but model currently supports version %s",
-		expr.Name, expr.Constraint, expr.Version, supported.Version,
-	)
+	return featureError(expr.Name, featDescr, errStr)
 }
 
 // satisfyCompositeExpr checks whether the feature set contents satisfy the
@@ -108,7 +101,7 @@ func satisfyCompositeExpr(fs FeatureSet, expr chassumes.CompositeExpression, exp
 	// below which introduces yet another indentation level and instead
 	// emit a top-level descriptive message.
 	if exprTreeDepth == 0 {
-		return requirementsNotSatisfied("Charm feature requirements cannot be met:", errList)
+		return requirementsNotSatisfied("Charm cannot be deployed because:", errList)
 	}
 
 	switch expr.Type() {

core/assumes/sat_checker_test.go

@@ -30,13 +30,9 @@ assumes:
 `)
 
 	expErr := `
-Charm feature requirements cannot be met:
+Charm cannot be deployed because:
   - charm requires feature "storage" (version >= 2.19.43) but model currently supports version 2.19.42
-
-Feature descriptions:
-  - "storage": create and manipulate storage primitives
-
-For additional information please see: https://juju.is/docs/olm/supported-features`[1:]
+`[1:]
 
 	err := fs.Satisfies(exprTree)
 	c.Assert(err, jc.Satisfies, IsRequirementsNotSatisfiedError, gc.Commentf("expected to get a RequirementsNotSatisfied error"))
@@ -57,19 +53,14 @@ assumes:
 `)
 
 	expErr := `
-Charm feature requirements cannot be met:
+Charm cannot be deployed because:
   - charm requires at least one of the following:
-    - charm requires feature "k8s-api" (version >= 42.0.0) but model currently supports version 1.18.0
+    - charm requires Kubernetes version >= 42.0.0, model is running on version 1.18.0
     - charm requires feature "random-feature-a" but model does not support it
   - charm requires all of the following:
     - charm requires feature "block-storage" but model does not support it
-    - charm requires feature "juju" (version >= 3.0.0) but model currently supports version 2.19.42
-
-Feature descriptions:
-  - "juju": version of Juju running on the controller
-  - "k8s-api": access to the kubernetes API
-
-For additional information please see: https://juju.is/docs/olm/supported-features`[1:]
+    - charm requires Juju version >= 3.0.0, model has version 2.19.42
+`[1:]
 
 	err := fs.Satisfies(exprTree)
 	c.Assert(err, jc.Satisfies, IsRequirementsNotSatisfiedError, gc.Commentf("expected to get a RequirementsNotSatisfied error"))
@@ -98,22 +89,17 @@ assumes:
 `)
 
 	expErr := `
-Charm feature requirements cannot be met:
+Charm cannot be deployed because:
   - charm requires feature "storage" (version >= 42.0.0) but model currently supports version 2.19.42
   - charm requires all of the following:
-    - charm requires feature "juju" (version >= 3.0.0) but model currently supports version 2.19.42
+    - charm requires Juju version >= 3.0.0, model has version 2.19.42
     - charm requires at least one of the following:
       - charm requires feature "random-feature-b" but model does not support it
       - charm requires feature "random-feature-c" but model does not support it
   - charm requires at least one of the following:
     - charm requires feature "random-feature-d" but model does not support it
     - charm requires feature "random-feature-e" but model does not support it
-
-Feature descriptions:
-  - "juju": version of Juju running on the controller
-  - "storage": create and manipulate storage primitives
-
-For additional information please see: https://juju.is/docs/olm/supported-features`[1:]
+`[1:]
 
 	err := fs.Satisfies(exprTree)
 	c.Assert(err, jc.Satisfies, IsRequirementsNotSatisfiedError, gc.Commentf("expected to get a RequirementsNotSatisfied error"))

state/stateenvirons/features.go

@@ -30,11 +30,7 @@ func SupportedFeatures(model Model, newEnviron environs.NewEnvironFunc) (assumes
 	}
 
 	agentVersion, _ := modelConf.AgentVersion()
-	fs.Add(assumes.Feature{
-		Name:        "juju",
-		Description: assumes.UserFriendlyFeatureDescriptions["juju"],
-		Version:     &agentVersion,
-	})
+	fs.Add(assumes.JujuFeature(agentVersion))
 
 	// Access the environment associated with the model and query any
 	// substrate-specific features that might be available.