Merge pull request #842 from terascope/k8s-improvements-2

K8s improvements 2
terascope · Oct 2, 2018 · 182242c · 182242c
2 parents 540dccb + a41132e
commit 182242c
Show file tree

Hide file tree

Showing 7 changed files with 184 additions and 68 deletions.
diff --git a/examples/k8s/Makefile b/examples/k8s/Makefile
@@ -96,11 +96,8 @@ destroy: ## delete k8s deployments and services
 	kubectl delete --namespace $(NAMESPACE) deployments,jobs,services,pods -l app=teraslice
 	kubectl delete --namespace $(NAMESPACE) configmap teraslice-master || echo "* it is okay..."
 	kubectl delete --namespace $(NAMESPACE) configmap teraslice-worker || echo "* it is okay..."
-	curl -fsS -XDELETE "${ES_URL}/terak8s*" || echo '* it is okay'
+	curl -fsS -XDELETE "${ES_URL}/ts-dev1*" || echo '* it is okay'
 	tjm reset ./example-job.json || echo '* it is okay'
-	tjm reset ./example-job-labels.json || echo '* it is okay'
-	tjm reset ./example-job-resource.json || echo '* it is okay'
-	tjm reset ./example-job-volume.json || echo '* it is okay'
 
 destroy-all: destroy deauth deelasticsearch delete-namespace ## delete ALL things including the namespace
 
@@ -111,9 +108,6 @@ rebuild: destroy setup ## destroys then re-runs things
 register: ## creates asset and registers job
 	tjm asset --deploy -c $(TERASLICE_MASTER_URL) || echo '* it is okay'
 	tjm register -c $(TERASLICE_MASTER_URL) ./example-job.json
-	tjm register -c $(TERASLICE_MASTER_URL) ./example-job-labels.json
-	tjm register -c $(TERASLICE_MASTER_URL) ./example-job-resource.json
-	tjm register -c $(TERASLICE_MASTER_URL) ./example-job-volume.json
 
 example: ## runs example job
 	yes | tjm asset --replace -c $(TERASLICE_MASTER_URL) || echo '* it is okay'

diff --git a/packages/job-components/src/formats.ts b/packages/job-components/src/formats.ts
@@ -51,6 +51,38 @@ export const formats : Format[] = [
             return val;
         },
     } as Format,
+    {
+        name: 'elasticsearch_Name',
+        validate(val: any) {
+            if (val.length > 255) {
+                throw new Error(`value: ${val} should not exceed 255 characters`);
+            }
+
+            if (_.startsWith(val, '_')
+                || _.startsWith(val, '-')
+                || _.startsWith(val, '+')) {
+                throw new Error(`value: ${val} should not start with _, -, or +`);
+            }
+
+            if (val === '.' || val === '..') {
+                throw new Error(`value: ${val} should not equal . or ..`);
+            }
+
+            // NOTE: the \\\\ is necessary to match a single \ in this case
+            const badChar = new RegExp('[#*?"<>|/\\\\]');
+            if (badChar.test(val)) {
+                throw new Error(`value: ${ val } should not contain any invalid characters: #*?"<>|/\\`);
+            }
+
+            const upperRE = new RegExp('[A-Z]');
+            if (upperRE.test(val)) {
+                throw new Error(`value: ${ val } should be lower case`);
+            }
+        },
+        coerce(val) {
+            return val;
+        },
+    } as Format,
 ];
 
 export function addFormats() {

diff --git a/packages/job-components/test/formats-spec.ts b/packages/job-components/test/formats-spec.ts
@@ -95,4 +95,148 @@ describe('Convict Formats', () => {
             optional.validate(undefined);
         }).not.toThrowError();
     });
+
+    describe('elasticsearch_Name', () => {
+        it('should be defined', () => {
+            const esName = getSchema('elasticsearch_Name');
+            if (!esName) {
+                expect(esName).not.toBeUndefined();
+                return;
+            }
+
+            expect(esName.name).toBeDefined();
+            expect(typeof esName.validate).toEqual('function');
+        });
+
+        it('should work for common index names', () => {
+            const esName = getSchema('elasticsearch_Name');
+            expect(() => {
+                // @ts-ignore
+                esName.validate('data-2018-01-01');
+            }).not.toThrowError();
+            expect(() => {
+                // @ts-ignore
+                esName.validate('data-2018-01-01.01');
+            }).not.toThrowError();
+
+        });
+
+        it('should not exceed 255 characters', () => {
+            const esName = getSchema('elasticsearch_Name');
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a'.repeat(256));
+            }).toThrow(/^value: .* should not exceed 255 characters/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a'.repeat(255));
+            }).not.toThrowError();
+        });
+
+        it('should not contain any of: #\\\/*?"<>|', () => {
+            const esName = getSchema('elasticsearch_Name');
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a#a');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a\\a');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a/a');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a*a');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a?a');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a"a');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a<a');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a>a');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a|a');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+
+            expect(() => {
+                // @ts-ignore
+                esName.validate('|aa');
+            }).toThrow(/^value: .* should not contain any invalid characters/);
+        });
+
+        it('should not start with _, -, or +', () => {
+            const esName = getSchema('elasticsearch_Name');
+
+            expect(() => {
+                // @ts-ignore
+                esName.validate('_foo');
+            }).toThrow(/^value: .* should not start with _, -, or +/);
+
+            expect(() => {
+                // @ts-ignore
+                esName.validate('-foo');
+            }).toThrow(/^value: .* should not start with _, -, or +/);
+
+            expect(() => {
+                // @ts-ignore
+                esName.validate('+foo');
+            }).toThrow(/^value: .* should not start with _, -, or +/);
+
+            expect(() => {
+                // @ts-ignore
+                esName.validate('a_foo');
+            }).not.toThrowError();
+
+        });
+
+        it('should not equal . or ..', () => {
+            const esName = getSchema('elasticsearch_Name');
+            expect(() => {
+                // @ts-ignore
+                esName.validate('.');
+            }).toThrow(/^value: .* should not equal . or ../);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('..');
+            }).toThrow(/^value: .* should not equal . or ../);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('.foo');
+            }).not.toThrowError();
+            expect(() => {
+                // @ts-ignore
+                esName.validate('..foo');
+            }).not.toThrowError();
+        });
+
+        it('should be lower case', () => {
+            const esName = getSchema('elasticsearch_Name');
+            expect(() => {
+                // @ts-ignore
+                esName.validate('ASDF');
+            }).toThrow(/^value: .* should be lower case/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('asdF');
+            }).toThrow(/^value: .* should be lower case/);
+            expect(() => {
+                // @ts-ignore
+                esName.validate('asdf');
+            }).not.toThrowError();
+        });
+    });
 });
diff --git a/packages/teraslice/lib/cluster/services/cluster/backends/kubernetes/index.js b/packages/teraslice/lib/cluster/services/cluster/backends/kubernetes/index.js
@@ -105,10 +105,9 @@ module.exports = function kubernetesClusterBackend(context, clusterMasterServer)
      * The hostname and port are used later by the workers to contact this
      * Execution Controller.
      * @param  {Object} execution        Object containing execution details
-     * @param  {String} recoverExecution ex_id of old execution to be recovered
      * @return {Promise}                 [description]
      */
-    function allocateSlicer(execution, recoverExecution) {
+    function allocateSlicer(execution) {
         const name = `teraslice-execution-controller-${execution.ex_id}`.substring(0, 63);
         const jobNameLabel = execution.name.replace(/[^a-zA-Z0-9_\-.]/g, '_').substring(0, 63);
 
@@ -127,8 +126,6 @@ module.exports = function kubernetesClusterBackend(context, clusterMasterServer)
         execution.slicer_port = _.get(exService, 'spec.ports[0].targetPort');
         execution.slicer_hostname = _.get(exService, 'metadata.name');
 
-        if (recoverExecution) execution.recover_execution = recoverExecution;
-
         const jobConfig = {
             name,
             assetsDirectory,

diff --git a/packages/teraslice/lib/cluster/services/execution.js b/packages/teraslice/lib/cluster/services/execution.js
@@ -398,13 +398,13 @@ module.exports = function module(context, { clusterMasterServer }) {
                         }))
                         .then(() => allocateWorkers(execution, execution.workers)
                             .catch((err) => {
-                            // this is to catch errors of allocateWorkers
-                            // if allocation fails, they are enqueued
-                                logger.error(`Workers failed to be allocated, they will be enqueued, error: ${parseError(err)}`);
+                                logger.error(`Failured to allocateWorkers ${execution.ex_id}, error: ${parseError(err)}`);
+                                return Promise.reject(err);
                             })))
                     .catch((err) => {
                         logger.error(`Failured to provision execution ${execution.ex_id}, error: ${parseError(err)}`);
-                        return setExecutionStatus(execution.ex_id, 'failed');
+                        const errMetaData = executionMetaData(null, parseError(err));
+                        return setExecutionStatus(execution.ex_id, 'failed', errMetaData);
                     })
                     .finally(() => {
                         allocatingExecution = false;
@@ -451,7 +451,7 @@ module.exports = function module(context, { clusterMasterServer }) {
                     return api;
                 }))
             .error((err) => {
-            // TODO: verify whats coming here
+                // TODO: verify whats coming here
                 if (_.get(err, 'body.error.reason') !== 'no such index') {
                     logger.error(`initialization failed loading state from Elasticsearch: ${err}`);
                 }

diff --git a/packages/teraslice/lib/config/schemas/system.js b/packages/teraslice/lib/config/schemas/system.js
@@ -73,7 +73,7 @@ const schema = {
     name: {
         doc: 'Name for the cluster itself, its used for naming log files/indices',
         default: 'teracluster',
-        format: 'required_String'
+        format: 'elasticsearch_Name'
     },
     state: {
         doc: 'Elasticsearch cluster where job state, analytics and logs are stored',

diff --git a/packages/teraslice/lib/utils/convict_utils.js b/packages/teraslice/lib/utils/convict_utils.js