Merge pull request #2330 from dcos/ph/fix/DCOS-16705-allow-unknown-va…

…lues-for-container DCOS-16705: fix(ServiceForm): allow unknown values for container images
dcos · Jul 20, 2017 · 38684b6 · 38684b6
2 parents bbbd887 + 6fd54cf
commit 38684b6
Show file tree

Hide file tree

Showing 5 changed files with 197 additions and 2 deletions.
diff --git a/plugins/services/src/js/reducers/serviceForm/Container.js b/plugins/services/src/js/reducers/serviceForm/Container.js
@@ -87,6 +87,11 @@ const containerJSONReducer = combineReducers({
     );
 
     const joinedPath = path && path.join(".");
+
+    if (type === SET && joinedPath === "container.docker") {
+      this.internalState = Object.assign({}, this.internalState, value);
+    }
+
     if (type === SET && joinedPath === "container.type") {
       this.containerType = value;
     }
@@ -312,6 +317,7 @@ module.exports = {
 
       return new Transaction(["container", "type"], value);
     },
+    simpleParser(["container", DOCKER.toLowerCase()]),
     simpleParser(["container", DOCKER.toLowerCase(), "image"]),
     simpleParser(["container", MESOS.toLowerCase(), "image"]),
     simpleParser(["container", DOCKER.toLowerCase(), "forcePullImage"]),

diff --git a/plugins/services/src/js/reducers/serviceForm/Docker.js b/plugins/services/src/js/reducers/serviceForm/Docker.js
@@ -11,6 +11,7 @@ function getContainerSettingsReducer(name) {
     if (joinedPath === "container.type" && Boolean(value)) {
       this.networkType = value;
     }
+
     if (type === SET && joinedPath === `container.docker.${name}`) {
       this.value = Boolean(value);
     }

diff --git a/plugins/services/src/js/reducers/serviceForm/JSONReducers/Containers.js b/plugins/services/src/js/reducers/serviceForm/JSONReducers/Containers.js
@@ -86,6 +86,9 @@ function containersParser(state) {
       memo.push(new Transaction(["containers", index, "name"], item.name));
     }
 
+    if (item.image) {
+      memo.push(new Transaction(["containers", index, "image"], item.image));
+    }
     if (item.image && item.image.id) {
       memo.push(
         new Transaction(["containers", index, "image", "id"], item.image.id)
@@ -344,6 +347,10 @@ module.exports = {
       this.endpoints = [];
     }
 
+    if (this.image == null) {
+      this.image = {};
+    }
+
     if (this.volumeMounts == null) {
       this.volumeMounts = [];
     }
@@ -473,9 +480,14 @@ module.exports = {
       );
     }
 
+    if (type === SET && joinedPath === `containers.${index}.image`) {
+      newState[index].image = this.image = Object.assign({}, this.image, value);
+    }
+
     if (type === SET && joinedPath === `containers.${index}.image.id`) {
-      newState[index] = Object.assign({}, newState[index], {
-        image: { id: value, kind: "DOCKER" }
+      newState[index].image = this.image = Object.assign({}, this.image, {
+        id: value,
+        kind: "DOCKER"
       });
       if (value === "") {
         delete newState[index].image;

diff --git a/plugins/services/src/js/reducers/serviceForm/JSONReducers/__tests__/Containers-test.js b/plugins/services/src/js/reducers/serviceForm/JSONReducers/__tests__/Containers-test.js
@@ -79,6 +79,105 @@ describe("Containers", function() {
           }
         ]);
       });
+
+      it("creates a complete image object", function() {
+        const batch = [
+          new Transaction(
+            ["containers"],
+            {
+              image: {
+                id: "nginx",
+                kind: "DOCKER",
+                pullConfig: {
+                  some: "value"
+                }
+              }
+            },
+            ADD_ITEM
+          ),
+          new Transaction(
+            ["containers", 0, "image"],
+            {
+              id: "nginx",
+              kind: "DOCKER",
+              pullConfig: {
+                some: "value"
+              }
+            },
+            SET
+          ),
+          new Transaction(["containers", 0, "image", "id"], "nginx", SET)
+        ].reduce(function(batch, transaction) {
+          return batch.add(transaction);
+        }, new Batch());
+
+        expect(batch.reduce(Containers.JSONReducer.bind({}))).toEqual([
+          {
+            name: "container-1",
+            resources: { cpus: 0.1, mem: 128 },
+            image: {
+              id: "nginx",
+              kind: "DOCKER",
+              pullConfig: {
+                some: "value"
+              }
+            }
+          }
+        ]);
+      });
+
+      it("creates a complete image object without loosing unknown", function() {
+        const batch = [
+          new Transaction(
+            ["containers"],
+            {
+              image: {
+                id: "nginx",
+                kind: "DOCKER",
+                pullConfig: {
+                  some: "value"
+                }
+              }
+            },
+            ADD_ITEM
+          ),
+          new Transaction(
+            ["containers", 0, "image"],
+            {
+              id: "nginx",
+              kind: "DOCKER",
+              pullConfig: {
+                some: "value"
+              }
+            },
+            SET
+          ),
+          new Transaction(["containers", 0, "image", "id"], "", SET),
+          new Transaction(
+            ["containers", 0, "artifacts"],
+            { uri: "http://mesosphere.io" },
+            ADD_ITEM
+          ),
+          new Transaction(["containers", 0, "image", "id"], "nginx", SET)
+        ].reduce(function(batch, transaction) {
+          return batch.add(transaction);
+        }, new Batch());
+
+        expect(batch.reduce(Containers.JSONReducer.bind({}))).toEqual([
+          {
+            name: "container-1",
+            resources: { cpus: 0.1, mem: 128 },
+            artifacts: [],
+            image: {
+              id: "nginx",
+              kind: "DOCKER",
+              pullConfig: {
+                some: "value"
+              }
+            }
+          }
+        ]);
+      });
     });
 
     describe("endpoints", function() {
@@ -899,5 +998,49 @@ describe("Containers", function() {
         ]);
       });
     });
+
+    it("stores complete image object", function() {
+      expect(
+        Containers.JSONParser({
+          containers: [
+            {
+              image: {
+                id: "nginx",
+                kind: "DOCKER",
+                pullConfig: {
+                  some: "value"
+                }
+              }
+            }
+          ]
+        })
+      ).toEqual([
+        new Transaction(
+          ["containers"],
+          {
+            image: {
+              id: "nginx",
+              kind: "DOCKER",
+              pullConfig: {
+                some: "value"
+              }
+            }
+          },
+          ADD_ITEM
+        ),
+        new Transaction(
+          ["containers", 0, "image"],
+          {
+            id: "nginx",
+            kind: "DOCKER",
+            pullConfig: {
+              some: "value"
+            }
+          },
+          SET
+        ),
+        new Transaction(["containers", 0, "image", "id"], "nginx", SET)
+      ]);
+    });
   });
 });
diff --git a/plugins/services/src/js/reducers/serviceForm/__tests__/Container-test.js b/plugins/services/src/js/reducers/serviceForm/__tests__/Container-test.js
@@ -1130,6 +1130,39 @@ describe("Container", function() {
     });
   });
 
+  describe("#Unknown Values", function() {
+    it("keep docker unknown values", function() {
+      expect(
+        Container.JSONParser({
+          container: {
+            docker: {
+              image: "nginx",
+              pullConfig: {
+                some: "value"
+              }
+            }
+          }
+        })
+          .reduce(function(batch, transaction) {
+            return batch.add(transaction);
+          }, new Batch())
+          .reduce(Container.JSONReducer.bind({}), {})
+      ).toEqual({
+        portMappings: null,
+        docker: {
+          image: "nginx",
+          forcePullImage: null,
+          privileged: null,
+          pullConfig: {
+            some: "value"
+          }
+        },
+        type: "MESOS",
+        volumes: []
+      });
+    });
+  });
+
   describe("Volumes", function() {
     it("should return an empty array if no volumes are set", function() {
       const batch = new Batch();