e2e TestWorkloadRef fails / LabelSelector is "null" & should be optional? #1675
Assignees
Labels
bug
Something isn't working
Comments
6 tasks
|
Could you try syncing up with the latest master? the test is passed for me |
|
Thank you @huikang . Can you add logging and see if your I also cleared out my The same error as before shows up in the e2e run: The controller logs show the error as above but before they also complain about a missing deployment
|
|
Alright - I did another test with minikube - which lets the e2e test succeed: And in a different terminal run the e2e test successfully After this I added a quick logging line diff --git a/rollout/temlateref.go b/rollout/temlateref.go
index 58cdf5d7..5e1dd0a1 100644
--- a/rollout/temlateref.go
+++ b/rollout/temlateref.go
@@ -247,6 +247,9 @@ func (r *informerBasedTemplateResolver) updateRolloutsReferenceAnnotation(obj in
if updated {
// update the annotation causes the rollout to be requeued and the template will be resolved to the referred
// workload during next reconciliation
+ if ro.Spec.Selector == nil {
+ log.Errorf(">>> Selector should not be nil! %v", ro)
+ }
ro.Spec.Template.Spec.Containers = []corev1.Container{}
_, err := r.argoprojclientset.ArgoprojV1alpha1().Rollouts(ro.Namespace).Update(context.TODO(), ro, v1.UpdateOptions{})
if err != nil {and sure enough, at the timestamp of the e2e the So - I guess minikube (and others?) are just more lenient and ignore such a 'broken' |
|
@derjust , thanks for the info. So the problem is reproducible in EKS. |
|
Yes - though i don't know if the problem is EKS (rejecting the |
|
I think we should use k8s patch to change generation annotation, instead of update. The update might fail due to invalid fields and also due to concurrent modification. Created PR that changes code to use patch #1678 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
Following the
docs/CONTRIBUTING.mddocumentation onmaster47d59faon OSX, the
TestWorkloadRefe2e fails - usingmake test-e2e E2E_TEST_OPTIONS="-testify.m ^TestWorkloadRef$"for all further descriptions below:(For context, i found this while working on my PR #1577)
Diagnostics
What version of Argo Rollouts are you running?
MasterI suspect this is an issue with my setup but i can't point to where it is:
Also all the other software is installed via Homebrew:
What am I missing?
Two observations:
First observation
Running
make codegeninmastercreates a diff - which i didn't expect:all changes are of this style:
fileDescriptor_XXXXXXXXXXX_Unmarshal()/XXX_Marshal()methodsWith & without this diff though I looked at the error itself:
Second observation
When the e2e test fails, the controller output is
with some additional logging from
The error message turns into
I'm not sure if the
selectorshould be set or not for the test.But my understanding is that
"null"is marshaled to theRollouts().Update()call - which is for sure not what is desired.Altering the type to make the
selectoroptional seems to fix the issue:as now the all tests pass.
But that seems to be the wrong solution?! Is this even an optional field?
My kubeconfig is set to use EKS/K8s 1.19
All those changes are also available in branch
MasterFixMessage from the maintainers:
Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.
The text was updated successfully, but these errors were encountered: