1. Introduction
- 1.1 Enviroment
- 1.2 Problem Description
2. problem analysis
- 2.1 the core process of this test case
- 2.2 Analyzing the code
  - 2.2.1 Important Information
  - 2.2.2 Source Confirmation
3. summary
4. Proposed modification

1. Introduction

1.1 Enviroment

golang
```
go version go1.13.4 darwin/amd64
```
1.2 Problem Description

We made some changes to PytochOperator types.go, which added some properties to status:

common.JobStatus is Anonymous filed

type PytorchJob struct {
    ...
    Status PytorchStatus `json:"status,omitempty"`
}

type PytorchStatus struct {
    common.JobStatus
    StatusForAdjust string `json:"statusForAdjust,omitempty"`
    ....
}

When testing the TestDeletePodsAndServices test case of job_test.go, we have been unable to pass. Compared to Pytorch-Operator, we have only changed the Status field in this part.
After carefully reading the test case information, we found important information. The following code is to add Success Condition to tc.job, and later delete Pods and Service during controller.go synchronization.
- After the following function, tc.job can successfully add Success Condition, and then convert to Unstructured form, and then add into job.Indexer, these can be performed normally
```
//job_test.go
err := updatePytorchJobConditions(tc.job, common.JobSucceeded, pytorchJobSucceededReason, "")
if err != nil {
    t.Errorf("Append job condition error: %v", err)
}
unstructured, err := testutil.ConvertPytorchJobToUnstructured(tc.job)
if err != nil {
    t.Errorf("Failed to convert the PytorchJob to Unstructured: %v", err)
}

if err := jobIndexer.Add(unstructured); err != nil {
    t.Errorf("Failed to add job to jobIndexer: %v", err)
}
```
- In the synchronous job process, we use the key (confirm that there is no problem) to retrieve the job from the Indexer. The job can be retrieved normally, but the Status Condition is nil, we expect to include Success Condition. Therefore, deletePodsAndServices will not be executed later, causing the test case to fail.
```
func (pc *PytorchController) syncPytorchJob(key string) (bool, error) {
    ...
    namespace, name, err := cache.SplitMetaNamespaceKey(key)
    ...
    sharedJob, err := pc.getPytorchJobFromName(namespace, name)
}
```
Why does PytorchJob Status Condition is nil after being transformed by unstructed?

2. problem analysis

2.1 the core process of this test case

new PytorchJob->"add Success Condition"->"convert the PytorchJob to Unstructured"->"add to job Indexer"->get“PytorchJob Unstructured object”with key from Indexer-> "Convert from Unstructured to PytorchJob"-> “Synchronized PytorchJob”
Important suspects Unstructured conversion error Indexer save error

2.2 Analyzing the code

2.2.1 Important Information

pc.getPytorchJobFromName (namespace, name) will call the FromUnstructured function. We print the information u taken from the Indexer and PytorchJob information after the conversion
- The information retrieved by the Indexer includes the Success status (Indexer is correct), but the PytorchJob does not have the Success status after the conversion is performed
- And the Unstructured Status shows that the common.JobStatus part of the json is the inline attribute (excluding JobStatus Struct), but the PytorchJob receives it not through the inline form (including JobStatus Struct, from "{{[] map []
  } 0 0}} ”can be seen)
  //vendor/k8s.io/apimachinery/pkg/runtime/convertet.go func (c *unstructuredConverter) FromUnstructured(u map[string]interface{}, obj interface{}) error { t := reflect.TypeOf(obj) value := reflect.ValueOf(obj) .... fmt.Println(reflect.ValueOf(u)) err := fromUnstructured(reflect.ValueOf(u), value.Elem()) fmt.Println(value.Elem()) ... }

map[kind:PytorchJob metadata:map[creationTimestamp:<nil> name:test-pytorchjob namespace:default] spec:map[block:<nil> cleanPodPolicy:All maxGPU:4 minGPU:4 pytorchReplicaSpecs:map[Master:map[replicas:1 template:map[metadata:map[creationTimestamp:<nil>] spec:map[containers:[map[args:[Fake Fake] image:test-image-for-kubeflow-pytorch-operator:latest name:pytorch ports:[map[containerPort:23456 name:pytorchjob-port]] resources:map[]]]]]] Worker:map[replicas:4 template:map[metadata:map[creationTimestamp:<nil>] spec:map[containers:[map[args:[Fake Fake] image:test-image-for-kubeflow-pytorch-operator:latest name:pytorch ports:[map[containerPort:23456 name:pytorchjob-port]] resources:map[]]]]]]] topologyGPU:<nil>] status:map[conditions:[map[lastTransitionTime:2019-12-14T09:02:53Z lastUpdateTime:2019-12-14T09:02:53Z reason:PytorchJobSucceeded status:True type:Succeeded]] replicaStatuses:<nil>]]

{{PytorchJob } {test-pytorchjob  default    0 0001-01-01 00:00:00 +0000 UTC <nil> <nil> map[] map[] [] nil [] } {<nil> <nil> 0xc0002bb0d0 <nil> 0xc000608cf8 0xc000608d08 <nil> <nil> map[Master:0xc000139600 Worker:0xc000139b80]} {{[] map[] <nil> <nil> <nil>}  0 0}}

2.2.2 Source Confirmation

FromUnstructured（） will call function structFromUnstructured（）

When the conversion to common.JobStatus is performed, the else part of the code will be executed (I have Printed fieldInfo.name is JobStatus), and the function will take the value of the key "JobStatus" from the sv map according to JobStatus, which is obviously not found. Therefore, a zero value is set for the JobStatus, so the PytorchJob Condition we finally obtained is nil

//vendor/k8s.io/apimachinery/pkg/runtime/convertet.go
func structFromUnstructured(sv, dv reflect.Value) error {
st, dt := sv.Type(), dv.Type()
if st.Kind() != reflect.Map {
    return fmt.Errorf("cannot restore struct from: %v", st.Kind())
}

for i := 0; i < dt.NumField(); i++ {
    fieldInfo := fieldInfoFromField(dt, i)
    fv := dv.Field(i)
    if len(fieldInfo.name) == 0 {
        // This field is inlined.
        if err := fromUnstructured(sv, fv); err != nil {
            return err
        }
    } else {
        value := unwrapInterface(sv.MapIndex(fieldInfo.nameValue))
        if value.IsValid() {
            if err := fromUnstructured(value, fv); err != nil {
                return err
            }
        } else {
            fv.Set(reflect.Zero(fv.Type()))
        }
    }
}
return nil
}

status:map[conditions:[map[lastTransitionTime:2019-12-14T09:02:53Z lastUpdateTime:2019-12-14T09:02:53Z reason:PytorchJobSucceeded status:True type:Succeeded]] replicaStatuses:<nil>]]

3. summary

The main reason is that when Struct anonymouse filed is converted by json, it is inline form by default (does not include common.JobStatus structure information), but golang reflect Struct anonymouse filed contains structure information, so the conversion from Unstructed to PytorchJob cannot be correctly converted.

4. Proposed modification

We need to handle the case where anonymous filed is included in Struct and the field is converted to Json with inline mode .

//vendor/k8s.io/apimachinery/pkg/runtime/convertet.go
for i := 0; i < dt.NumField(); i++ {
    fieldInfo := fieldInfoFromField(dt, i)
    fv := dv.Field(i)
    // if we cannot found fieldInfo.name of Anonymous field in json,the json is inline mode for this Anonymous field
    _, ok := sv.Interface().(map[string]interface{})[fieldInfo.name]
    if len(fieldInfo.name) == 0 || dt.Field(i).Anonymous && !ok {
        // if len(fieldInfo.name) == 0 {
        // This field is inlined.
        if err := fromUnstructured(sv, fv); err != nil {
            return err
        }
    } else {
        ...
    }
}

After modification, we successfully passed the test case

kubeflow / pytorch-operator

Unstructured converted to Pytorch Job Anonymous field error when json uses inline mode #234