[CORD-3180] Setting app_id as mandatory and handling 409 error

Change-Id: Ie712d1e8e6bef402c63893f070ebb5de9f01e518
diff --git a/docs/README.md b/docs/README.md
index 082d1e0..8e82320 100644
--- a/docs/README.md
+++ b/docs/README.md
@@ -21,6 +21,68 @@
 For more informations about the models, please refer to the
 [xproto](https://github.com/opencord/onos-service/blob/master/xos/synchronizer/models/onos.xproto) definition
 
+### Example TOSCA
+
+This is an example TOSCA recipe you can use to install an application
+and add some configuration to it:
+
+```yaml
+tosca_definitions_version: tosca_simple_yaml_1_0
+imports:
+  - custom_types/onosapp.yaml
+  - custom_types/onosservice.yaml
+  - custom_types/serviceinstanceattribute.yaml
+description: Configures fabric switches and related ports
+topology_template:
+  node_templates:
+    service#onos:
+      type: tosca.nodes.ONOSService
+      properties:
+        name: onos
+        must-exist: true
+
+
+    # Local app
+    dhcp:
+      type: tosca.nodes.ONOSApp
+      properties:
+        name: dhcp
+        app_id: org.onosproject.dhcp
+      requirements:
+        - owner:
+            node: service#onos
+            relationship: tosca.relationships.BelongsToOne
+
+    # Remote app
+    cord-config:
+      type: tosca.nodes.ONOSApp
+      properties:
+        app_id: org.opencord.cord-config
+        name: cord-config
+        url: https://oss.sonatype.org/content/repositories/public/org/opencord/cord-config/1.4.0-SNAPSHOT/cord-config-1.4.0-20180604.071543-275.oar
+        version: 1.4.0.SNAPSHOT
+      requirements:
+        - owner:
+            node: service#onos
+            relationship: tosca.relationships.BelongsToOne
+
+    # CORD-Configuration
+    cord-config-attr:
+      type: tosca.nodes.ServiceInstanceAttribute
+      properties:
+        name: /onos/v1/network/configuration/apps/org.opencord.olt
+        value: >
+          {
+            "kafka" : {
+              "bootstrapServers" : "cord-kafka-kafka.default.svc.cluster.local:9092"
+            }
+          }
+      requirements:
+        - service_instance:
+            node: cord-config
+            relationship: tosca.relationships.BelongsToOne
+```
+
 ## Synchronization workflow
 
 ### ONOSService
@@ -42,5 +104,5 @@
 
 > ONOS Applications can be activated (if they already present in the container),
 > in that case you just need to provide the `app_id`, or they can be installed from a remote `.oar`,
-> in which case you just need to provide an `url` 
+> in which case you need to also provide an `url` and a `version`
  
\ No newline at end of file
diff --git a/samples/apps.yaml b/samples/apps.yaml
index d923788..4594586 100644
--- a/samples/apps.yaml
+++ b/samples/apps.yaml
@@ -44,6 +44,7 @@
     cord-config:
       type: tosca.nodes.ONOSApp
       properties:
+        app_id: org.opencord.cord-config
         name: cord-config
         url: https://oss.sonatype.org/content/repositories/public/org/opencord/cord-config/1.4.0-SNAPSHOT/cord-config-1.4.0-20180604.071543-275.oar
         version: 1.4.0.SNAPSHOT
diff --git a/xos/synchronizer/models/models.py b/xos/synchronizer/models/models.py
new file mode 100644
index 0000000..9c01b5b
--- /dev/null
+++ b/xos/synchronizer/models/models.py
@@ -0,0 +1,35 @@
+# Copyright 2017-present Open Networking Foundation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from xos.exceptions import XOSValidationError
+
+from models_decl import ONOSApp_decl
+from models_decl import ONOSService_decl
+
+class ONOSApp(ONOSApp_decl):
+    class Meta:
+        proxy = True
+
+    def save(self, *args, **kwargs):
+
+        if self.url and not self.version:
+            raise XOSValidationError("If you specify and url, you also need to specify a version. ONOSApp:  %s" % self.name)
+
+        super(ONOSApp, self).save(*args, **kwargs)
+
+
+class ONOSService(ONOSService_decl):
+   class Meta:
+        proxy = True 
+
diff --git a/xos/synchronizer/models/onos.xproto b/xos/synchronizer/models/onos.xproto
index 726702d..041d7e4 100644
--- a/xos/synchronizer/models/onos.xproto
+++ b/xos/synchronizer/models/onos.xproto
@@ -1,15 +1,15 @@
-option kind="onos";
 option app_label = "onos";
 option name="onos";
+option legacy="True";
 
 message ONOSApp (ServiceInstance){
     option verbose_name="ONOS Application";
     option owner_class_name="ONOSService";
 
-    optional string app_id = 1 [db_index = False, null = True, blank = False];
+    required string app_id = 1 [db_index = False, null = False, blank = False];
     required string dependencies = 2 [help_text="Comma separated list of required applications", db_index = False, null = True, blank = True];
     optional string url = 3 [help_text="URL at which the application is available, if it needs to be downloaded", db_index = False, null = True, blank = False];
-    required string version = 4 [db_index = False, null = True, blank = False];
+    optional string version = 4 [db_index = False, null = True, blank = False];
 }
 
 message ONOSService (Service){
diff --git a/xos/synchronizer/steps/sync_onos_app.py b/xos/synchronizer/steps/sync_onos_app.py
index 6288243..cc30645 100644
--- a/xos/synchronizer/steps/sync_onos_app.py
+++ b/xos/synchronizer/steps/sync_onos_app.py
@@ -1,4 +1,3 @@
-
 # Copyright 2017-present Open Networking Foundation
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -91,11 +90,13 @@
             o.version = request.json()["version"]
 
     def check_app_installed(self, o, onos_url, onos_basic_auth):
+        log.debug("Checking if app is installed", app=o.app_id)
         url = '%s/onos/v1/applications/%s' % (onos_url, o.app_id)
         request = requests.get(url, auth=onos_basic_auth)
 
         if request.status_code == 200:
             if "version" in request.json() and o.version == request.json()["version"]:
+                log.debug("App is installed", app=o.app_id)
                 return True
             else:
                 # uninstall the application
@@ -106,23 +107,19 @@
             return False
         else:
             log.error("Request failed", response=request.text)
-            raise Exception("Failed to read application %s from ONOS: %s" % (url, request.text))
+            raise Exception("Failed to read application %s from ONOS aaa: %s" % (url, request.text))
 
     def install_app(self, o, onos_url, onos_basic_auth):
-        log.info("Installing app from url %s" % o.url)
+        log.info("Installing app from url %s" % o.url, app=o.app_id, version=o.version)
 
-        # check is the already installed app is the correct version (if it has no app_id is not installed)
-        is_installed = False
-        if o.app_id and o.app_id is not None:
-            is_installed = self.check_app_installed(o, onos_url, onos_basic_auth)
+        # check is the already installed app is the correct version
+        is_installed = self.check_app_installed(o, onos_url, onos_basic_auth)
 
         if is_installed:
             # if the app is already installed we don't need to do anything
+            log.info("App is installed, skipping install", app=o.app_id)
             return
 
-        if not o.version or o.version is None:
-            # TODO move this validation in the model.py (if the url is there version must there and app_id must not)
-            raise Exception('You need to specify a version')
         data = {
             'activate': True,
             'url': o.url
@@ -130,18 +127,22 @@
         url = '%s/onos/v1/applications' % onos_url
         request = requests.post(url, json=data, auth=onos_basic_auth)
 
+        if request.status_code == 409:
+            log.info("App was already installed", app=o.app_id, test=request.text)
+            return
+
         if request.status_code != 200:
             log.error("Request failed", response=request.text)
             raise Exception("Failed to add application %s to ONOS: %s" % (url, request.text))
 
-        o.app_id = request.json()["name"]
+        log.debug("App from url %s installed" % o.url, app=o.app_id, version=o.version)
 
         url = '%s/onos/v1/applications/%s' % (onos_url, o.app_id)
         request = requests.get(url, auth=onos_basic_auth)
 
         if request.status_code != 200:
             log.error("Request failed", response=request.text)
-            raise Exception("Failed to read application %s from ONOS: %s" % (url, request.text))
+            raise Exception("Failed to read application %s from ONOS: %s while checking correct version" % (url, request.text))
         else:
             if o.version != request.json()["version"]:
                 raise Exception("The version of %s you installed (%s) is not the same you requested (%s)" % (o.app_id, request.json()["version"], o.version))
diff --git a/xos/synchronizer/steps/test_sync_onos_app.py b/xos/synchronizer/steps/test_sync_onos_app.py
index 4d64783..bd63b99 100644
--- a/xos/synchronizer/steps/test_sync_onos_app.py
+++ b/xos/synchronizer/steps/test_sync_onos_app.py
@@ -209,7 +209,7 @@
 
         self.onos_app.url = 'http://onf.org/maven/...'
         self.onos_app.version = "1.13.1"
-        self.onos_app.app_id = None
+        self.onos_app.app_id = "org.onosproject.vrouter"
 
         expected = {
             'activate': True,
@@ -221,9 +221,10 @@
                additional_matcher=functools.partial(match_json, expected),
                json=self.vrouter_app_response)
 
-        m.get("http://onos-url:8181/onos/v1/applications/org.onosproject.vrouter",
-              status_code=200,
-              json=self.vrouter_app_response)
+        m.get("http://onos-url:8181/onos/v1/applications/org.onosproject.vrouter", [
+            {'status_code': 404, 'text': "foo"},
+            {'status_code': 200, 'json': self.vrouter_app_response}
+        ])
 
         self.si.serviceinstanceattribute_dict = {}
 
@@ -231,7 +232,7 @@
             mock_si.return_value = [self.si]
             self.sync_step().sync_record(self.onos_app)
         self.assertTrue(m.called)
-        self.assertEqual(m.call_count, 2)
+        self.assertEqual(m.call_count, 3)
         self.assertEqual(self.onos_app.app_id, self.vrouter_app_response["name"])
 
     @requests_mock.Mocker()
@@ -280,7 +281,7 @@
 
         self.onos_app.url = 'http://onf.org/maven/...'
         self.onos_app.version = "1.14.2"
-        self.onos_app.app_id = None
+        self.onos_app.app_id = "org.onosproject.vrouter"
 
         expected = {
             'activate': True,
@@ -292,9 +293,10 @@
                additional_matcher=functools.partial(match_json, expected),
                json=self.vrouter_app_response)
 
-        m.get("http://onos-url:8181/onos/v1/applications/org.onosproject.vrouter",
-              status_code=200,
-              json=self.vrouter_app_response)
+        m.get("http://onos-url:8181/onos/v1/applications/org.onosproject.vrouter", [
+            {'status_code': 404, 'text': "foo"},
+            {'status_code': 200, 'json': self.vrouter_app_response}
+        ])
 
         self.si.serviceinstanceattribute_dict = {}
 
@@ -304,11 +306,33 @@
             self.sync_step().sync_record(self.onos_app)
 
         self.assertTrue(m.called)
-        self.assertEqual(m.call_count, 2)
+        self.assertEqual(m.call_count, 3)
         self.assertEqual(self.onos_app.app_id, self.vrouter_app_response["name"])
         self.assertEqual(e.exception.message, "The version of org.onosproject.vrouter you installed (1.13.1) is not the same you requested (1.14.2)")
 
     @requests_mock.Mocker()
+    def test_handle_409(self, m):
+        """
+        A 409 "Application Already installed" response is not an error. This should not happen as we check if the app is installed.
+        """
+
+        self.onos_app.url = 'http://onf.org/maven/...'
+        self.onos_app.version = "1.14.2"
+        self.onos_app.app_id = "org.onosproject.vrouter"
+
+        m.post("/onos/v1/applications",
+               status_code=409)
+
+        step = self.sync_step()
+        with patch.object(step, "check_app_installed") as mock_check_installed:
+            mock_check_installed.return_value = False
+
+            step.sync_record(self.onos_app)
+
+        self.assertTrue(m.called)
+        self.assertEqual(m.call_count, 1)
+
+    @requests_mock.Mocker()
     def test_config_delete(self, m):
         m.delete("http://onos-url:8181%s" % self.onos_app_attribute.name,
                status_code=204)