[Runtime] Resolved references missing from desired Spec during rm.Create call

[vijtrip2]

Currently inside reconciler.go, the sequence of operations during resource creation is

1. Resolve resource references
2. Perform AWSResourceManager.ReadOne operation
3. NotFound Error returned by ReadOne call
4. Call setResourceManaged i.e. Add finalizer to the resource
5. Perform AWSResourceManager.Create call.

The resolved references are important for building ReadOne and Create requests and the problem with above sequence is that the kc.Patch call from setResourceManaged(step 4) method restores the desired Spec from etcd and the resolved references(step 1) get reset.

The solution should make sure that resolved references are present in desired spec while performing AWSResourceManager.Create call

[vijtrip2]

There are multiple solution for this problem.

I propose the solution to be performing reference resolution again before the Create call. This is a simple solution without any side-effects. This solution introduces a very simple logic, reuses existing function code, and very low probability of introducing new bugs. The only disadvantage with this approach is the extra calls to API-server for resolving the reference but important thing to notice is that the rm.ResolveReferences method call right before rm.Create will mostly only get executed once in complete resource lifecycle i.e. during the creation time.

Alternate Solutions:

1. Mark the resource managed before resolving references. This solution is optimized and can work in most cases but it has a side-effect associated with prematurely adding the finalizer. Consider following scenario, The resource is marked managed and then there are Errors in ReadOne call(except NotFound error) , then reconciler will retrying until terminal condition is set. Now if the customer wish to delete the resource, they cannot because the ReadOne before Delete call will also fail and keep requeueing the delete request. This issue does not occur currently because Finalizer is added only after the ReadOne call. Since reference resolution is needed for ReadOne call, resource cannot be marked as managed before reference resolution.

2. Keep a copy of ReferenceResolvedSpec before setResourceManaged call and set desiredSpec to ReferenceResolvedSpec after setResourceManaged call. This approach is also simple and matches how ACK reconciler keeps the Status copy, but the logic is more complicated than the preferred solution above. This will save extra API-server calls but require changes in AWSResource interface to add SetSpec method.

3. Similar to alternate solution 2, but we mark resource as managed using copy of desired object and copy the metadata from updated object(passed in parameter of setResourceManaged) into desired object. Same pros/cons as solution 2.

@jaypipes , @RedbackThomson & @a-hilaly , please let me know your thoughts on the solutions mentioned in this comment.

[RedbackThomson]

I'm happy with that proposed solution. Copy around metadata and specs between various versions of the resources is not obvious or particularly understandable. I think if we can justify why we need to resolve the references within a comment on the line above it then that's the simplest solution. No one way doors here, so happy to make a move and see how it turns out.