Bug 1865779

Summary: [mhc] timeout field without units(h,m,s) shoud not be allowed to be stored
Product: OpenShift Container Platform Reporter: Milind Yadav <miyadav>
Component: Cloud ComputeAssignee: Joel Speed <jspeed>
Cloud Compute sub component: Other Providers QA Contact: Milind Yadav <miyadav>
Status: CLOSED ERRATA Docs Contact:
Severity: low    
Priority: low    
Version: 4.6   
Target Milestone: ---   
Target Release: 4.6.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-10-27 16:23:11 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Milind Yadav 2020-08-04 07:08:58 UTC
[mhc] Error message not proper

cluster version - 4.6.0-0.nightly-2020-08-04-002217


steps:
1.Create a mhc with below yaml 
apiVersion: machine.openshift.io/v1beta1
kind: MachineHealthCheck
metadata:
  creationTimestamp: "2020-02-14T09:47:08Z"
  generation: 1
  name: mhc2
  namespace: openshift-machine-api
  nodeStartupTimeout: 12m
spec:
  selector: {}
  maxUnhealthy: 2
  unhealthyConditions:
    - 
      status: "False"
      timeout: 3
      type: Ready
      
2. run oc create -f <mhc.yaml>
      [miyadav@miyadav debug]$ oc create -f mhc.yaml 
Actual: The MachineHealthCheck "mhc2" is invalid: spec.unhealthyConditions.timeout: Invalid value: "integer": spec.unhealthyConditions.timeout in body must be of type string: "integer"
Expected : The MachineHealthCheck "mhc2" is invalid: spec.unhealthyConditions.timeout: Invalid value: "": spec.unhealthyConditions.timeout in body should match '^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h)*)+$'


Additional info:
As the error message says invalid value "integer" , ...must be of type.. "integer"
It might be confusing .
Expected message I mentioned , can be reviewed for giving more info on what is exactly wrong in timeout value used in yaml

Comment 1 Joel Speed 2020-08-04 14:51:32 UTC
I don't think there is any way to force the validation to recognise the int as a string and produce the better error message. We may not be able to resolve this.
I will do some research into this,

Comment 2 Joel Speed 2020-08-04 15:42:11 UTC
I asked in slack and I don't think we can resolve this issue, a type error will always trump a formatting error

https://kubernetes.slack.com/archives/C0EG7JC6T/p1596554610279700

Comment 3 Milind Yadav 2020-08-05 04:45:13 UTC
Hi @joel , 
Here is the error we get when using "3" in quotes , so as you suggested providing those details here - 

E0804 10:57:22.394358 1 reflector.go:178] sigs.k8s.io/controller-runtime/pkg/cache/internal/informers_map.go:224: Failed to list *v1beta1.MachineHealthCheck: v1beta1.MachineHealthCheckList.ListMeta: v1.ListMeta.TypeMeta: Kind: Items: []v1beta1.MachineHealthCheck: v1beta1.MachineHealthCheck.Spec: v1beta1.MachineHealthCheckSpec.UnhealthyConditions: []v1beta1.UnhealthyCondition: v1beta1.UnhealthyCondition.Type: Timeout: unmarshalerDecoder: time: missing unit in duration 3, error found in #10 byte of ...|meout":"3","type":"R|..., bigger context ...|ealthyConditions":[

{"status":"False","timeout":"3","type":"Ready"}

So we can modify the bug to handle this issue of storing the value which shouldnt happen instead of error message..
Let me know if you have any comments on this ..

Comment 4 Joel Speed 2020-08-05 12:46:44 UTC
I don't mind which we do. The issue of storing the integer cannot be resolved as per the discussion I had in slack.

I have a fix open for the format to require units already, if you update the description of the bug we can use this. Alternatively we close this as cantfix and create a new bug for the missing formats, wdyt?

Comment 5 Milind Yadav 2020-08-06 04:11:53 UTC
Updated the description as per comment #4

Comment 8 Milind Yadav 2020-08-10 04:11:36 UTC
VERIFIED ON - 

Cluster version is 4.6.0-0.nightly-2020-08-07-202945

Steps:
1.Create a mhc below yaml 
apiVersion: machine.openshift.io/v1beta1
kind: MachineHealthCheck
metadata:
  creationTimestamp: "2020-02-14T09:47:08Z"
  generation: 1
  name: mhc1
  namespace: openshift-machine-api
  nodeStartupTimeout: 12m
spec:
  selector: {}
  maxUnhealthy: 2
  unhealthyConditions:
    - 
      status: "False"
      timeout: "3"
      type: Ready
~                  

Expected and Actual :
[miyadav@miyadav debug]$ oc create -f mhc.yaml  --config awskube 
The MachineHealthCheck "mhc1" is invalid: spec.unhealthyConditions.timeout: Invalid value: "": spec.unhealthyConditions.timeout in body should match '^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$'

2.Repeat with timeout as "3h" and 3h 

Expected and actual : mhc created successfully

Comment 10 errata-xmlrpc 2020-10-27 16:23:11 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (OpenShift Container Platform 4.6 GA Images), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:4196