fix for some AWs that do not complete

[asm582]

Issue link

#657

What changes have been made

Changes to addqueue method which exists only after informer and etcd has status.

Verification steps

I have manually run scale tests and confirmed all AWs complete

(base) abhishekmalvankar@Abhisheks-MBP perf-test % kubectl get appwrappers --all-namespaces -o json | jq '.items[] | select(.status.Succeeded != 1) | {namespace: .metadata.namespace, name: .metadata.name, status: .status}' (base) abhishekmalvankar@Abhisheks-MBP perf-test % 

Checks

• I've made sure the tests are passing.
• Testing Strategy
  • Unit tests
  • Manual tests
  • Testing is not required for this change

[jbusche]

I'm hoping I did my PR onto your PR right, but I made a few small adjustments to the perf.sh script that seems to be working well. I rebuilt the mcad image based on PR 658 and it's running very well, not seeing any hanging appwrappers - I've run up to 300 so far.

As a control, I also ran the same perf.sh against the codeflare-operator:v1.0.0-rc.1 release and that one does well with 5 and 20 appwrappers, but is consistently hanging with 50 appwrappers. The jobs all show complete, but the appwrappers are fighting to write to etcd.

I think this PR is a big improvement, if we can just get the small perf.sh tweaks in there.

[jbusche]

It's looking really good @asm582, I've built the image and then done:

helm install mcad-controller --namespace kube-system --generate-name --set image.repository=quay.io/jbusche/mcad-controller --set image.tag=comp-stat-try2 

and then running your newly modified perf.sh and it's looking really good:

AppWrappers Total Time (seconds) MCAD Image 50 181 seconds Abishek PR 658 50 182 seconds Abishek PR 658 100 343 seconds Abishek PR 658 200 670 seconds Abishek PR 658 300 1011 seconds Abishek PR 658 

I'm also watching the appwrappers and it's cleanly finishing even the long runs, which was NOT the case in the regular rc.1 image.

watch "kubectl get appwrappers --all-namespaces -o json | jq '.items[] | select(.status.Succeeded != 1) | {namespace: .metadata.namespace, name: .metadata.name, status: .status}' |wc -l" 0 

[jbusche]

LGTM, tested great - performs well and the perf script is greatly improved, looking at appwrapper status instead of job status.

[jbusche]

Shoot - this change isn't working right - getting the error message:

Error from server (BadRequest): Unable to find "workload.codeflare.dev/v1beta1, Resource=appwrappers" that match label selector "", field selector "status.successful=1": field label not supported: status.successful 

But since the jobstatus later in the while loop uses jobs the perf.sh script still completes.

[metalcycling]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jbusche, metalcycling

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [metalcycling]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment