Skip to content

test/e2e: Wrap catalog template image adjustment test in eventually blocks #2397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

timflannagan
Copy link
Member

Update the "adding catalog template adjusts image used" test block and
wrap flake-sensitive methods in eventually handlers. This also attempts
to fix a flake that occasionally pops up in e2e runs:

• Failure [60.555 seconds]
Catalog represents a store of bundles which OLM can use to install Operators
/home/tflannag/Documents/openshift/operator-framework-olm/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go:35
  adding catalog template adjusts image used [It]
  /home/tflannag/Documents/openshift/operator-framework-olm/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go:1036

  Timed out after 60.001s.
  Expected success, but got an error:
      <*errors.errorString | 0xc0009e47b0>: {
          s: "resource name may not be empty",
      }
      resource name may not be empty

Update the sourceName -> source.GetName() when firing off that
problematic GET call.

@openshift-ci
Copy link

openshift-ci bot commented Oct 5, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: timflannagan
To complete the pull request process, please assign ecordell after the PR has been reviewed.
You can assign the PR to them by writing /assign @ecordell in a comment when ready.

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

Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci bot requested review from dinhxuanvu and njhale October 5, 2021 16:06
…andlers

Update the "adding catalog template adjusts image used" test block and
wrap flake-sensitive methods in eventually handlers. This also attempts
to fix a flake that occasionally pops up in e2e runs:

```log
• Failure [60.555 seconds]
Catalog represents a store of bundles which OLM can use to install Operators
/home/tflannag/Documents/openshift/operator-framework-olm/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go:35
  adding catalog template adjusts image used [It]
  /home/tflannag/Documents/openshift/operator-framework-olm/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go:1036

  Timed out after 60.001s.
  Expected success, but got an error:
      <*errors.errorString | 0xc0009e47b0>: {
          s: "resource name may not be empty",
      }
      resource name may not be empty
```

Update the `sourceName` -> `source.GetName()` when firing off that
problematic GET call.

Signed-off-by: timflannagan <[email protected]>
@timflannagan timflannagan force-pushed the update-adding-catalog-template-image-e2e-test branch from 80a586a to e046611 Compare October 8, 2021 23:23
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 15, 2021
@openshift-ci
Copy link

openshift-ci bot commented Oct 15, 2021

@timflannagan: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@exdx
Copy link
Member

exdx commented Oct 18, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2021
Comment on lines +1071 to +1077
source, err = crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Get(context.TODO(), source.GetName(), metav1.GetOptions{})
if err != nil {
return err
}
// create an annotation using the kube templates
source.SetAnnotations(map[string]string{catalogsource.CatalogImageTemplateAnnotation: fmt.Sprintf("quay.io/olmtest/catsrc-update-test:%s.%s.%s", catalogsource.TemplKubeMajorV, catalogsource.TemplKubeMinorV, catalogsource.TemplKubePatchV)})

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revisiting this as it's been sitting for a while: poking at this locally, it looks like the real issue is we're overwriting this shared source variable in the GET and UPDATE calls. Speculatively, it looks like anytime the update call fails, the source variable will be essentially nil:

catalogsource info: catalog-9t8pz/openshift-operators
catalogsource info: &v1alpha1.CatalogSource{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:v1.ObjectMeta{Name:"catalog-9t8pz", GenerateName:"", Namespace:"openshift-operators", SelfLink:"", UID:"1151883a-39f2-44a0-9d42-8c2e28505b47", ResourceVersion:"32421", Generation:1, CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63772780625, loc:(*time.Location)(0x46b9aa0)}}, DeletionTimestamp:(*v1.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string{"olm.catalogSource":"catalog-9t8pz"}, Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Finalizers:[]string(nil), ClusterName:"", ManagedFields:[]v1.ManagedFieldsEntry{v1.ManagedFieldsEntry{Manager:"e2e.test", Operation:"Update", APIVersion:"operators.coreos.com/v1alpha1", Time:(*v1.Time)(0xc0001be498), FieldsType:"FieldsV1", FieldsV1:(*v1.FieldsV1)(0xc0001be480), Subresource:""}}}, Spec:v1alpha1.CatalogSourceSpec{SourceType:"grpc", Priority:0, ConfigMap:"", Address:"", Image:"quay.io/olmtest/catsrc-update-test:old", UpdateStrategy:(*v1alpha1.UpdateStrategy)(nil), Secrets:[]string(nil), DisplayName:"", Description:"", Publisher:"", Icon:v1alpha1.Icon{Data:"", MediaType:""}}, Status:v1alpha1.CatalogSourceStatus{Message:"", Reason:"", LatestImageRegistryPoll:(*v1.Time)(nil), ConfigMapResource:(*v1alpha1.ConfigMapResourceReference)(nil), RegistryServiceStatus:(*v1alpha1.RegistryServiceStatus)(nil), GRPCConnectionState:(*v1alpha1.GRPCConnectionState)(nil), Conditions:[]v1.Condition(nil)}}
catalogsource info: /
catalogsource info: /
...

One solution is to just avoid overwriting this source resource by suppressing the variable returned from the update call. I tested out this variation locally and I wasn't able to reproduce this issue anymore.

@timflannagan timflannagan deleted the update-adding-catalog-template-image-e2e-test branch November 17, 2021 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants