Added Update and Replace APIs for Service custom values

acceptance/configurations_test.go

@@ -467,7 +467,7 @@ var _ = Describe("Configurations", LConfiguration, func() {
 			// edit the configuration ...
 
 			out, err := env.Epinio("", "configuration", "update", configurationName1,
-				"--remove", "username",
+				"--unset", "username",
 				"--set", "user=ci/cd",
 			)
 			Expect(err).ToNot(HaveOccurred(), out)

acceptance/services_test.go

@@ -1100,4 +1100,92 @@ var _ = Describe("Services", LService, func() {
 			})
 		})
 	})
+
+	Describe("service update", func() {
+		var appName string
+		var service string
+
+		BeforeEach(func() {
+			settings, err := env.GetSettingsFrom(testenv.EpinioYAML())
+			Expect(err).ToNot(HaveOccurred())
+
+			appName = catalog.NewAppName()
+			service = catalog.NewServiceName()
+			containerImageURL := "splatform/sample-app"
+			serviceHostname := strings.Replace(settings.API, `https://epinio`, service, 1)
+
+			env.MakeContainerImageApp(appName, 1, containerImageURL)
+
+			By("creating service instance: " + service)
+			out, err := env.Epinio("", "service", "create", catalogService.Meta.Name, service,
+				"--chart-value", "ingress.enabled=true",
+				"--chart-value", "ingress.hostname="+serviceHostname,
+				"--chart-value", "nesting.here.hello=world",
+				"--wait")
+			Expect(err).ToNot(HaveOccurred(), out)
+
+			out, err = env.Epinio("", "service", "bind", service, appName)
+			Expect(err).ToNot(HaveOccurred(), out)
+
+			// Wait for the app restart from binding the service to settle
+			Eventually(func() string {
+				out, err := env.Epinio("", "app", "list")
+				Expect(err).ToNot(HaveOccurred(), out)
+				return out
+			}, "5m").Should(
+				HaveATable(
+					WithHeaders("NAME", "CREATED", "STATUS", "ROUTES", "CONFIGURATIONS", "STATUS DETAILS"),
+					WithRow(appName, WithDate(), "1/1", appName+".*", ".*", ""),
+				),
+			)
+		})
+
+		It("it edits the service, and restarts the app", func() {
+			By("editing instance: " + service)
+			out, err := env.Epinio("", "service", "update", service,
+				"--wait",
+				"--unset", "ingress.enabled",
+				"--unset", "ingress.hostname",
+				"--set", "nesting.here.hello=user",
+			)
+			Expect(err).ToNot(HaveOccurred(), out)
+			Expect(out).To(ContainSubstring("Update Service"))
+			Expect(out).To(
+				HaveATable(
+					WithHeaders("PARAMETER", "OP", "VALUE"),
+					WithRow("ingress.enabled", "remove", ""),
+					WithRow("ingress.hostname", "remove", ""),
+					WithRow("nesting.here.hello", "add\\/change", "user"),
+				),
+			)
+			Expect(out).To(ContainSubstring("Service Changes Saved"))
+
+			// Confirm the changes ...
+			By("checking instance: " + service)
+			out, err = env.Epinio("", "service", "show", service)
+			Expect(err).ToNot(HaveOccurred(), out)
+
+			Expect(out).To(ContainSubstring("Showing Service"))
+			Expect(out).To(
+				HaveATable(
+					WithHeaders("KEY", "VALUE"),
+					WithRow("nesting.here.hello", "user"),
+				),
+			)
+			Expect(out).ToNot(ContainSubstring("ingress"))
+
+			// Wait for app to resettle ...
+			By("waiting for app to resettle: " + appName)
+			Eventually(func() string {
+				out, err := env.Epinio("", "app", "list")
+				Expect(err).ToNot(HaveOccurred(), out)
+				return out
+			}, "5m").Should(
+				HaveATable(
+					WithHeaders("NAME", "CREATED", "STATUS", "ROUTES", "CONFIGURATIONS", "STATUS DETAILS"),
+					WithRow(appName, WithDate(), "1/1", appName+".*", ".*", ""),
+				),
+			)
+		})
+	})
 })

docs/references/api/swagger.json

@@ -1663,6 +1663,39 @@
           }
         }
       },
+      "put": {
+        "description": "Replace the named `Service` in the `Namespace` as per the instructions in the body",
+        "tags": [
+          "service"
+        ],
+        "operationId": "ServiceReplace",
+        "parameters": [
+          {
+            "type": "string",
+            "name": "Namespace",
+            "in": "path",
+            "required": true
+          },
+          {
+            "type": "string",
+            "name": "Service",
+            "in": "path",
+            "required": true
+          },
+          {
+            "name": "Body",
+            "in": "body",
+            "schema": {
+              "$ref": "#/definitions/ServiceReplaceRequest"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "$ref": "#/responses/ServiceReplaceResponse"
+          }
+        }
+      },
       "delete": {
         "tags": [
           "service"
@@ -1695,6 +1728,39 @@
             "$ref": "#/responses/ServiceDeleteResponse"
           }
         }
+      },
+      "patch": {
+        "description": "Update the named `Service` in the `Namespace` as per the instructions in the body",
+        "tags": [
+          "service"
+        ],
+        "operationId": "ServiceUpdate",
+        "parameters": [
+          {
+            "type": "string",
+            "name": "Namespace",
+            "in": "path",
+            "required": true
+          },
+          {
+            "type": "string",
+            "name": "Service",
+            "in": "path",
+            "required": true
+          },
+          {
+            "name": "Body",
+            "in": "body",
+            "schema": {
+              "$ref": "#/definitions/ServiceUpdateRequest"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "$ref": "#/responses/ServiceUpdateResponse"
+          }
+        }
       }
     },
     "/namespaces/{Namespace}/services/{Service}/bind": {
@@ -3008,6 +3074,20 @@
       },
       "x-go-package": "github.com/epinio/epinio/pkg/api/core/v1/models"
     },
+    "ServiceReplaceRequest": {
+      "description": "ServiceReplaceRequest represents and contains the data needed to\nreplace a service instance (i.e. the custom value keys)",
+      "type": "object",
+      "properties": {
+        "settings": {
+          "$ref": "#/definitions/ChartValueSettings"
+        },
+        "wait": {
+          "type": "boolean",
+          "x-go-name": "Wait"
+        }
+      },
+      "x-go-package": "github.com/epinio/epinio/pkg/api/core/v1/models"
+    },
     "ServiceStatus": {
       "type": "string",
       "x-go-package": "github.com/epinio/epinio/pkg/api/core/v1/models"
@@ -3022,6 +3102,27 @@
       },
       "x-go-package": "github.com/epinio/epinio/pkg/api/core/v1/models"
     },
+    "ServiceUpdateRequest": {
+      "description": "ServiceUpdateRequest represents and contains the data needed to\nupdate a service instance (add/change, and remove custom value keys)",
+      "type": "object",
+      "properties": {
+        "edit": {
+          "$ref": "#/definitions/ChartValueSettings"
+        },
+        "remove": {
+          "type": "array",
+          "items": {
+            "type": "string"
+          },
+          "x-go-name": "Remove"
+        },
+        "wait": {
+          "type": "boolean",
+          "x-go-name": "Wait"
+        }
+      },
+      "x-go-package": "github.com/epinio/epinio/pkg/api/core/v1/models"
+    },
     "StageRef": {
       "description": "StageRef references a staging run by ID, currently randomly generated\nfor each POST to the staging endpoint",
       "type": "object",
@@ -3418,6 +3519,12 @@
     "ServicePortForwardResponse": {
       "description": ""
     },
+    "ServiceReplaceResponse": {
+      "description": "",
+      "schema": {
+        "$ref": "#/definitions/Response"
+      }
+    },
     "ServiceShowResponse": {
       "description": "",
       "schema": {
@@ -3430,6 +3537,12 @@
         "$ref": "#/definitions/Response"
       }
     },
+    "ServiceUpdateResponse": {
+      "description": "",
+      "schema": {
+        "$ref": "#/definitions/Response"
+      }
+    },
     "StagingCompleteResponse": {
       "description": "",
       "schema": {

internal/api/v1/application/deploy.go

@@ -12,6 +12,8 @@
 package application
 
 import (
+	"context"
+
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
@@ -111,3 +113,34 @@
 	})
 	return nil
 }
+
+// Redeploy does not serve a specific handler. It is used by the configuration and service
+// update/replace handlers to restart the active set of the named applications. Quiescent
+// applications are ignored. This is their means of forcing the applications bound to the changed
+// configuration/service to pick up these changes and use them.
+func Redeploy(ctx context.Context, cluster *kubernetes.Cluster, namespace string, appNames []string) apierror.APIErrors {
+	username := requestctx.User(ctx).Username
+
+	for _, appName := range appNames {
+		app, err := application.Lookup(ctx, cluster, namespace, appName)
+		if err != nil {
+			return apierror.InternalError(err)
+		}
+
+		// Restart workload, if any
+		if app.Workload != nil {
+			// TODO :: This plain restart is different from all other restarts
+			// (scaling, ev change, bound configurations change) ... The deployment
+			// actually does not change, at all. A resource the deployment
+			// references/uses changed, i.e. the configuration. We still have to
+			// trigger the restart somehow, so that the pod mounting the
+			// configuration remounts it for the new/changed keys.
+			_, apiErr := deploy.DeployAppWithRestart(ctx, cluster, app.Meta, username, "")
+			if apiErr != nil {
+				return apiErr
+			}
+		}
+	}
+
+	return nil
+}

internal/api/v1/configuration/replace.go

@@ -13,10 +13,9 @@
 
 import (
 	"github.com/epinio/epinio/helpers/kubernetes"
-	"github.com/epinio/epinio/internal/api/v1/deploy"
+	apiapp "github.com/epinio/epinio/internal/api/v1/application"
 	"github.com/epinio/epinio/internal/api/v1/response"
 	"github.com/epinio/epinio/internal/application"
-	"github.com/epinio/epinio/internal/cli/server/requestctx"
 	"github.com/epinio/epinio/internal/configurations"
 	apierror "github.com/epinio/epinio/pkg/api/core/v1/errors"
 	"github.com/epinio/epinio/pkg/api/core/v1/models"
@@ -40,9 +39,7 @@
 		if err.Error() == "configuration not found" {
 			return apierror.ConfigurationIsNotKnown(configurationName)
 		}
-		if err != nil {
-			return apierror.InternalError(err)
-		}
+		return apierror.InternalError(err)
 	}
 
 	var replaceRequest models.ConfigurationReplaceRequest
@@ -56,40 +53,20 @@
 		return apierror.InternalError(err)
 	}
 
-	// Perform restart on the candidates which are actually running
 	if restart {
-		username := requestctx.User(ctx).Username
-
 		// Determine bound apps, as candidates for restart.
 		appNames, err := application.BoundAppsNamesFor(ctx, cluster, namespace, configurationName)
 		if err != nil {
 			return apierror.InternalError(err)
 		}
 
-		for _, appName := range appNames {
-			app, err := application.Lookup(ctx, cluster, namespace, appName)
-			if err != nil {
-				return apierror.InternalError(err)
-			}
-
-			// Restart workload, if any
-			if app.Workload != nil {
-				// TODO :: This plain restart is different from all other restarts
-				// (scaling, ev change, bound configurations change) ... The deployment
-				// actually does not change, at all. A resource the deployment
-				// references/uses changed, i.e. the configuration. We still have to
-				// trigger the restart somehow, so that the pod mounting the
-				// configuration remounts it for the new/changed keys.
-				_, apierr := deploy.DeployAppWithRestart(ctx, cluster, app.Meta, username, "")
-				if apierr != nil {
-					return apierr
-				}
-			}
+		// Perform restart on the candidates which are actually running
+		apiErr := apiapp.Redeploy(ctx, cluster, namespace, appNames)
+		if apiErr != nil {
+			return apiErr
 		}
 	}
 
-	// Done
-
 	response.OK(c)
 	return nil
 }