Repository: arista-eosplus/ztpserver (language: python)

Issue:
[BUG] Downloading newer EOS images causes OOM on smaller switches
Hi there, 

When using the `install_image` action on ZTP'ing a brand new switch I'm getting OOM errors on Arista 7010 OOB switches.
This started happening around EOS version 4.26.3M, but it can also be early then that, I didn't try all versions.
My guess is that this is cause by the incremental growth of the EOS image size with each version and the fact that the ZTPserver uses a plain GET request to download the given EOS image. As far as I know by default Python request loads the response into memory first, before writing to disk and because the process is memory restricted this causes the process to be OOM killed.

See example error output below;
```
[admin@localhost flash]$ free -h
             total       used       free     shared    buffers     cached
Mem:          3.8G       2.6G       1.2G         0B        50M       806M
-/+ buffers/cache:       1.8G       2.0G
Swap:           0B         0B         0B
[admin@localhost flash]$ free -h
             total       used       free     shared    buffers     cached
Mem:          3.8G       2.6G       1.2G         0B        50M       806M
-/+ buffers/cache:       1.8G       2.0G
Swap:           0B         0B         0B
[admin@localhost flash]$ free -h
             total       used       free     shared    buffers     cached
Mem:          3.8G       2.6G       1.2G         0B        50M       806M
-/+ buffers/cache:       1.8G       2.0G
Swap:           0B         0B         0B
[admin@localhost flash]$ free -h
             total       used       free     shared    buffers     cached
Mem:          3.8G       2.6G       1.2G         0B        50M       806M
-/+ buffers/cache:       1.8G       2.0G
Swap:           0B         0B         0B
[admin@localhost flash]$ free -h
             total       used       free     shared    buffers     cached
Mem:          3.8G       2.7G       1.1G         0B        50M       806M
-/+ buffers/cache:       1.8G       2.0G
Swap:           0B         0B         0B
[admin@localhost flash]$ free -h
             total       used       free     shared    buffers     cached
Mem:          3.8G       2.7G       1.1G         0B        50M       806M
-/+ buffers/cache:       1.8G       2.0G
Swap:           0B         0B         0B
[admin@localhost flash]$ [81002.319709] Out of memory: Kill process 21008 (ReloadCauseAgen) score 919 or sacrifice child
Jan  5 14:11:19 localhost kernel: [81002.319709] Out of memory: Kill process 21008 (ReloadCauseAgen) score 919 or sacrifice chil[81002.546948] Killed process 21008 (ReloadCauseAgen) total-vm:454076kB, anon-rss:18704kB, file-rss:62344kB, shmem-rss:24kB
[81002.546948] memory usage:1.9% score:917130 oom_score_adj:900
d
Jan  5 14:11:19 localhost kernel: [81002.546948] Killed process 21008 (ReloadCauseAgen) total-vm:454076kB, anon-rss:18704kB, file-rss:62344kB, shmem-rss:24kB
Jan  5 14:11:19 localhost kernel: [81002.546948] memory usage:1.9% score:917130 oom_score_adj:900
[81003.194389] Out of memory: Kill process 21162 (python) score 526 or sacrifice child
Jan  5 14:11:20 [81003.289228] Killed process 21162 (python) total-vm:2193648kB, anon-rss:2152380kB, file-rss:8368kB, shmem-rss:0kB
[81003.289228] memory usage:52.6% score:525019 oom_score_adj:0
localhost kernel: [81003.194389] Out of memory: Kill process 21162 (python) score 526 or sacrifice child
Jan  5 14:11:20 localhost kernel: [81003.289228] Killed process 21162 (python) total-vm:2193648kB, anon-rss:2152380kB, file-rss:8368kB, shmem-rss:0kB
Jan  5 14:11:20 localhost kernel: [81003.289228] memory usage:52.6% score:525019 oom_score_adj:0
Jan  5 14:11:20 localhost ZeroTouch: %ZTP-4-EXEC_SCRIPT_SIGNALED: Config script exited with an uncaught signal. Signal code: Killed
Jan  5 14:11:20 localhost ZeroTouch: %ZTP-6-RETRY: Retrying Zero Touch Provisioning from the beginning (attempt 2423)
```

A solution would be to write the image to disk in chunks using request streaming instead of downloading the whole image into memory.


Discussion / hints:

Fix patch:
diff --git a/client/bootstrap b/client/bootstrap
index eb2e8509..b0f29a77 100755
--- a/client/bootstrap
+++ b/client/bootstrap
@@ -724,7 +724,7 @@ class Node(object):
         If 'path' is somewhere on flash and 'url' points back to
         SERVER, then the client will request the metadata for
         the resource from the server (in order to check whether there
-        is enogh disk space available). If 'url' points to a different
+        is enough disk space available). If 'url' points to a different
         server, then the 'content-length' header will be used for the
         disk space checks.
 
@@ -913,7 +913,7 @@ class Server(object):
 
     @classmethod
     def _http_request(cls, path=None, method='get', headers=None,
-                      payload=None, files=None):
+                      payload=None, files=None, local_file=None):
         if headers is None:
             headers = {}
         # Disable gzip, deflate so we can safely determine available space
@@ -930,6 +930,7 @@ class Server(object):
         else:
             full_url = path
 
+        response = None
         try:
             if method == 'get':
                 log('GET %s' % full_url)
@@ -945,19 +946,40 @@ class Server(object):
                                          headers=headers,
                                          files=request_files,
                                          timeout=HTTP_TIMEOUT)
+            elif method == 'stream':
+                log('STREAM %s - %s' % (full_url, local_file))
+                # Don't use streaming request in CI environment
+                if os.environ.get('EAPI_TEST') is not None:
+                    log('NOT STREAM FOR CI %s - %s' % (full_url, local_file))
+                    response = requests.get(full_url,
+                                            data=json.dumps(payload),
+                                            headers=headers,
+                                            files=request_files,
+                                            timeout=HTTP_TIMEOUT)
+                else:
+                    if not local_file:
+                        raise ZtpError('Cant STREAM EOS image file without'
+                                       ' file name and path')
+                    with requests.get(full_url, stream=True) as response:
+                        response.raise_for_status()
+                        with open(local_file, 'wb') as f:
+                            for chunk in response.iter_content(chunk_size=8192):
+                                f.write(chunk)
             else:
-                log('Unknown method %s' % method,
-                    error=True)
+                log('Unknown method %s' % method, error=True)
         except requests.exceptions.ConnectionError:
             raise ZtpError('server connection error')
 
         return response
 
-    def _get_request(self, url):
+    def _get_request(self, url, stream=False, local_file=None):
         # resource or action
         headers = {'content-type': CONTENT_TYPE_HTML}
-        result = self._http_request(url,
-                                    headers=headers)
+        if stream:
+            result = self._http_request(url, method='stream', headers=headers,
+                                        local_file=local_file)
+        else:
+            result = self._http_request(url, headers=headers)
         log('Server response to GET request: status=%s' % result.status_code)
 
         return (result.status_code,
@@ -1000,17 +1022,24 @@ class Server(object):
                 log('WARNING: flash disk usage will exceeed %s%% after '
                     'saving %s to %s' % (percent, url, path))
 
-        log('Writing %s...' % path)
+            log('File streamed earlier. No need to write content.')
 
-        # Save contents to file
-        try:
-            with open(path, 'wb') as result:
-                for chunk in contents.iter_content(chunk_size=1024):
-                    if chunk:
-                        result.write(chunk)
-                result.close()
-        except IOError as err:
-            raise ZtpError('unable to write %s: %s' % (path, err))
+        write_response_contents = False
+        if os.environ.get('EAPI_TEST') is not None:
+            log('File not streamed earlier in CI environment. Need to write content')
+            write_response_contents = True
+
+        if not path.startswith(FLASH) or write_response_contents:
+            log('Writing %s...' % path)
+            # Save contents to file
+            try:
+                with open(path, 'wb') as result:
+                    for chunk in contents.iter_content(chunk_size=1024):
+                        if chunk:
+                            result.write(chunk)
+                    result.close()
+            except IOError as err:
+                raise ZtpError('unable to write %s: %s' % (path, err))
 
         # Set permissions
         os.chmod(path, 0777)
@@ -1027,7 +1056,7 @@ class Server(object):
         if(status != HTTP_STATUS_OK or
            content != CONTENT_TYPE_JSON):
             raise ZtpUnexpectedServerResponseError(
-                'unexpected reponse from server (status=%s; content-type=%s)' %
+                'unexpected response from server (status=%s; content-type=%s)' %
                 (status, content))
 
         return (status, content, result)
@@ -1051,7 +1080,7 @@ class Server(object):
                           HTTP_STATUS_CONFLICT] or
            content != CONTENT_TYPE_HTML):
             raise ZtpUnexpectedServerResponseError(
-                'unexpected reponse from server (status=%s; content-type=%s)' %
+                'unexpected response from server (status=%s; content-type=%s)' %
                 (status, content))
         elif status == HTTP_STATUS_BAD_REQUEST:
             raise ZtpError('node not found on server (status=%s)' % status)
@@ -1077,7 +1106,7 @@ class Server(object):
                 (status == HTTP_STATUS_BAD_REQUEST and
                  content == CONTENT_TYPE_HTML)):
             raise ZtpUnexpectedServerResponseError(
-                'unexpected reponse from server (status=%s; content-type=%s)' %
+                'unexpected response from server (status=%s; content-type=%s)' %
                 (status, content))
         elif status == HTTP_STATUS_BAD_REQUEST:
             raise ZtpError('server-side topology check failed (status=%s)' %
@@ -1094,7 +1123,7 @@ class Server(object):
                 (status == HTTP_STATUS_NOT_FOUND and
                  content == CONTENT_TYPE_HTML)):
             raise ZtpUnexpectedServerResponseError(
-                'unexpected reponse from server (status=%s; content-type=%s)' %
+                'unexpected response from server (status=%s; content-type=%s)' %
                 (status, content))
         elif status == HTTP_STATUS_NOT_FOUND:
             raise ZtpError('action not found on server (status=%s)' % status)
@@ -1128,7 +1157,7 @@ class Server(object):
                 (status == HTTP_STATUS_INTERNAL_SERVER_ERROR and
                  content == CONTENT_TYPE_HTML)):
             raise ZtpUnexpectedServerResponseError(
-                'unexpected reponse from server (status=%s; content-type=%s)' %
+                'unexpected response from server (status=%s; content-type=%s)' %
                 (status, content))
         elif status == HTTP_STATUS_NOT_FOUND:
             raise ZtpError('metadata not found on server (status=%s)' %
@@ -1144,7 +1173,11 @@ class Server(object):
         if not urlparse.urlsplit(url).scheme:     # pylint: disable=E1103
             url = url_path_join(SERVER, url)
 
-        status, content, response = self._get_request(url)
+        if path.startswith(FLASH):
+            status, content, response = self._get_request(url, stream=True,
+                                                          local_file=path)
+        else:
+            status, content, response = self._get_request(url)
 
         if url.startswith(SERVER):
             if not ((status == HTTP_STATUS_OK and
@@ -1152,14 +1185,14 @@ class Server(object):
                     (status == HTTP_STATUS_NOT_FOUND and
                      content == CONTENT_TYPE_HTML)):
                 raise ZtpUnexpectedServerResponseError(
-                    'unexpected reponse from server for %s '
+                    'unexpected response from server for %s '
                     '(status=%s; content-type=%s)' %
                     (url, status, content))
         else:
             if not (status == HTTP_STATUS_OK or
                     status == HTTP_STATUS_NOT_FOUND):
                 raise ZtpUnexpectedServerResponseError(
-                    'unexpected reponse from server for %s '
+                    'unexpected response from server for %s '
                     '(status=%s; content-type=%s)' %
                     (url, status, content))
 
@@ -1405,20 +1438,17 @@ if __name__ == '__main__':
     try:
         main()
     except ZtpError as exception:
-        log('''Bootstrap process failed:
-   %s''' % str(exception),
-            error=True)
+        log('Bootstrap process failed: %s' % str(exception), error=True)
         _exit(1)
     except KeyboardInterrupt:
-        log('Bootstrap process keyboard-interrupted',
-            error=True)
+        log('Bootstrap process keyboard-interrupted', error=True)
         log(sys.exc_info()[0])
         log(traceback.format_exc())
         _exit(1)
     except Exception, exception:
-        log('''Bootstrap process failed because of unknown exception:
-   %s''' %
-            exception, error=True)
+        errStr = 'Bootstrap process failed because of unknown' \
+                 ' exception: %s' % exception
+        log(errStr, error=True)
         log(sys.exc_info()[0])
         log(traceback.format_exc())
         _exit(1)


Test patch:
diff --git a/test/client/client_test_lib.py b/test/client/client_test_lib.py
index 093341c7..8d37379f 100755
--- a/test/client/client_test_lib.py
+++ b/test/client/client_test_lib.py
@@ -387,7 +387,7 @@ def eapi_failure(self):
                self.return_code
 
     def unexpected_response_failure(self):
-        return 'unexpected reponse from server' in self.output and \
+        return 'unexpected response from server' in self.output and \
                self.return_code
 
     def node_not_found_failure(self):