Quota release e2e testing and Fits function fix

[metalcycling]

This PR includes testing for the quota release using Kuttl. The tests revealed a major problem with the Fits function in qm_lib_backend_with_quotasubt_mgr.go which prevents allocating quota to a consumer that has already been added. This is not the right behavior as a consumer can be added to the quota tree while it waits for quota to be available. The previous code prevented existing consumers from getting quota due to the fact that the already exists in the tree.

[z103cb]

@metalcycling I think this change is not necessary, and it decreases the ease of code comprehension.

It is not necessary because the call allocResponse, err := qm.quotaManagerBackend.AllocateForest(QuotaManagerForestName, consumerID) will have the same effect as proposed change implementation (see if forestController.IsConsumerAllocated(consumerID) {)

This change is not helping the code comprehension as:

• logging an error and continuing is a bad practice.
• if you do need to handle the error, it should probably be logged using the warning level.

Overall this change should be reverted as:

• it only makes code quality worse
• it doesn't fix the underlying issue (allocating consumers when they are already allocated)

The proper fix for the issue highlighted by the test-case would be to replace the code at line 530

	klog.V(4).Infof("[Fits] Sending quota allocation request: %#v ", consumerInfo)
	allocResponse, err := qm.quotaManagerBackend.AllocateForest(QuotaManagerForestName, consumerID)
	if err != nil {
		qm.removeConsumer(consumerID)
		klog.Errorf("[Fits] Error allocating consumer: %s/%s, err=%#v.", aw.Namespace, aw.Name, err)
		return false, nil, err.Error()
	}
	klog.V(4).Infof("[Fits] allocation response received. Quota Allocated: %t, Message: '%s', Preempted app wrappers count: %d", allocResponse.IsAllocated(),
		strings.TrimSpace(allocResponse.GetMessage()), len(allocResponse.GetPreemptedIds()))
	doesFit := allocResponse.IsAllocated()
	if !doesFit {
		qm.removeConsumer(consumerID)
		return doesFit, preemptIds, strings.TrimSpace(allocResponse.GetMessage())
	}
	preemptIds = qm.getAppWrappers(allocResponse.GetPreemptedIds())
	return doesFit, preemptIds, strings.TrimSpace(allocResponse.GetMessage())

The removeConsumer helper function code:

func (qm *QuotaManager) removeConsumer(consumerID string) {
	// removing the consumer to allow for the consumer to be added if and when
	// the function is called for the same app wrapper
	removed, err := qm.quotaManagerBackend.RemoveConsumer(consumerID)
	if err != nil {
		klog.Warningf("Failed to remove consumer %s, %#v", consumerID, err)
	}
	if !removed {
		klog.Warningf("Failed to remove consumer %s", consumerID)
	}
}

[z103cb]

The changes to the Fits function are needed.

[z103cb]

Nitpick, you should probably add a comment as to why are you deleting all the app wrappers that have previously been installed.

[z103cb]

Nitpick: I think that it would be safer to increase this value to at least 180, or remove it all together.

[metalcycling]

#397 implemented a better fix and added the test case in this PR, so no need for it anymore.