Bug 1558519

Summary:	Validation of config.yml values
Product:	[oVirt] ovirt-engine-metrics	Reporter:	Lukas Svaty <lsvaty>
Component:	Generic	Assignee:	Shirly Radco <sradco>
Status:	CLOSED NOTABUG	QA Contact:	Lukas Svaty <lsvaty>
Severity:	low	Docs Contact:
Priority:	unspecified
Version:	1.1.3.1	CC:	bugs, sradco
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	All
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2018-03-20 12:37:13 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	Metrics	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Lukas Svaty 2018-03-20 11:39:17 UTC

Description of problem:
Validation exists on ovirt_env_name only.

no validation in config.yml on this values:
1. viaq_metrics_store: foo

foo is considered a string, therefore "fasle" -> true
Only true/false should be accepted

2. openshift_deployment_type
should accept only origin/openshift-enterprise

3. no validation on fluentd_elasticsearch_host: 
In case of viaq_metrics_store: false there is no validation on this field if the address is acceptable, however, we will connect to the elasticsearch anyway, we should validate the connection is possible and it is listening on port 9200

4. ovirt_metrics_curator_delete_days
should accept only numbers

Comment 1 Shirly Radco 2018-03-20 12:37:13 UTC

.(In reply to Lukas Svaty from comment #0)
> Description of problem:
> Validation exists on ovirt_env_name only.
> 
> no validation in config.yml on this values:
> 1. viaq_metrics_store: foo

Not relevant anymore

> 
> foo is considered a string, therefore "fasle" -> true
> Only true/false should be accepted
> 
> 2. openshift_deployment_type
> should accept only origin/openshift-enterprise

Not relevant anymore
> 
> 3. no validation on fluentd_elasticsearch_host: 
> In case of viaq_metrics_store: false there is no validation on this field if
> the address is acceptable, however, we will connect to the elasticsearch
> anyway, we should validate the connection is possible and it is listening on
> port 9200

No validation is required.

> 
> 4. ovirt_metrics_curator_delete_days
> should accept only numbers

We dont need to add validation to every variable in the playbook. This is not ansible playbook best practice.

Comment 2 Lukas Svaty 2018-03-20 12:44:51 UTC

(In reply to Shirly Radco from comment #1)
> .(In reply to Lukas Svaty from comment #0)
> > Description of problem:
> > Validation exists on ovirt_env_name only.
> > 
> > no validation in config.yml on this values:
> > 1. viaq_metrics_store: foo
> 
> Not relevant anymore
> 
> > 
> > foo is considered a string, therefore "fasle" -> true
> > Only true/false should be accepted
> > 
> > 2. openshift_deployment_type
> > should accept only origin/openshift-enterprise
> 
> Not relevant anymore
> > 
> > 3. no validation on fluentd_elasticsearch_host: 
> > In case of viaq_metrics_store: false there is no validation on this field if
> > the address is acceptable, however, we will connect to the elasticsearch
> > anyway, we should validate the connection is possible and it is listening on
> > port 9200
> 
> No validation is required.

This is required, as we want to ensure fluentd is able to connect to elasticsearch, if not this is spamming the logs and cause a big amount of data filling the disk.

> 
> > 
> > 4. ovirt_metrics_curator_delete_days
> > should accept only numbers
> 

All of them are still relevant in version ovirt-engine-metrics-1.1.3.3-1.el7ev.noarch currently included in 4.2.2

> We dont need to add validation to every variable in the playbook. This is
> not ansible playbook best practice.

I agree, however I would suggest either dont validation anything or validate all of them to be consistend.