Skip to content
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

Submariner ServiceExport does not have unique condition types to wait for readiness #714

Closed
vthapar opened this issue Mar 23, 2022 · 4 comments · Fixed by #715
Closed

Submariner ServiceExport does not have unique condition types to wait for readiness #714

vthapar opened this issue Mar 23, 2022 · 4 comments · Fixed by #715
Assignees
Labels
bug Something isn't working size:small

Comments

@vthapar
Copy link
Contributor

vthapar commented Mar 23, 2022

What happened:
After creating service export with subctl, ther's no way to validate it with kubectl/oc wait command,
since Submariner ServiceExport does not have unique condition types to wait for readiness (all types are the same "Valid").

For example, on a working service export status it failed on:

$ oc wait serviceexport nginx-cl-bc --for=condition=Valid
error: timed out waiting for the condition on serviceexports/nginx-cl-bc

What you expected to happen:
If the service export is working, it should return something like:

multicluster.x-k8s.io/ServiceExportList condition met

How to reproduce it (as minimally and precisely as possible):
Create a service export:
$ oc get serviceexport
NAME AGE
nginx-cl-bc 3h5m

$ oc get serviceexport -o yaml
apiVersion: v1
items:

apiVersion: multicluster.x-k8s.io/v1alpha1
kind: ServiceExport
metadata:
creationTimestamp: "2022-03-21T08:25:40Z"
generation: 1
name: nginx-cl-bc
namespace: test-submariner
resourceVersion: "115495575"
selfLink: /apis/multicluster.x-k8s.io/v1alpha1/namespaces/test-submariner/serviceexports/nginx-cl-bc
uid: 7af4a9ae-24f6-4d1d-9ac5-100664e03dd9
status:
conditions:
lastTransitionTime: "2022-03-21T08:25:39Z"
message: Service doesn't have a global IP yet
reason: ServiceGlobalIPUnavailable
status: "False"
type: Valid
lastTransitionTime: "2022-03-21T08:25:39Z"
message: Awaiting sync of the ServiceImport to the broker
reason: AwaitingSync
status: "False"
type: Valid
lastTransitionTime: "2022-03-21T08:25:39Z"
message: Service was successfully synced to the broker
reason: ""
status: "True"
type: Valid
lastTransitionTime: "2022-03-21T09:59:32Z"
message: Service to be exported doesn't exist
reason: ServiceUnavailable
status: "False"
type: Valid
lastTransitionTime: "2022-03-21T09:59:32Z"
message: Service doesn't have a global IP yet
reason: ServiceGlobalIPUnavailable
status: "False"
type: Valid
lastTransitionTime: "2022-03-21T09:59:33Z"
message: Awaiting sync of the ServiceImport to the broker
reason: AwaitingSync
status: "False"
type: Valid
lastTransitionTime: "2022-03-21T09:59:35Z"
message: Service was successfully synced to the broker
reason: ""
status: "True"
type: Valid
lastTransitionTime: "2022-03-21T10:20:16Z"
message: Service to be exported doesn't exist
reason: ServiceUnavailable
status: "False"
type: Valid
lastTransitionTime: "2022-03-21T10:20:16Z"
message: Service doesn't have a global IP yet
reason: ServiceGlobalIPUnavailable
status: "False"
type: Valid
kind: List
metadata:
resourceVersion: ""
selfLink: ""
$ oc wait serviceexport nginx-cl-bc --for=condition=Valid
error: timed out waiting for the condition on serviceexports/nginx-cl-bc

Anything else we need to know?:

Environment:
Submariner version 0.12.0

  • Diagnose information (use subctl diagnose all):
  • Gather information (use subctl gather):
  • Cloud provider or hardware configuration:
  • Install tools:
  • Others:
@vthapar vthapar added the bug Something isn't working label Mar 23, 2022
@vthapar
Copy link
Contributor Author

vthapar commented Mar 23, 2022

Multiple conditions with Valid with some True and some False is what causes problems. We need a single Ready condition that should be set to True/False based on when readiness changes. Better would be for MCS API to provide unique condition types.

@vthapar vthapar added this to Backlog in Backlog via automation Mar 23, 2022
@tpantelis
Copy link
Contributor

tpantelis commented Mar 23, 2022

Multiple conditions with Valid with some True and some False is what causes problems. We need a single Ready condition that should be set to True/False based on when readiness changes. Better would be for MCS API to provide unique condition types.

It seems condition types are intended to be singletons, ie only retain the last state. I didn't realize that at the time and implemented it as a historical trace which I think also has some merit however I think we should utilize conditions as intended.

@vthapar
Copy link
Contributor Author

vthapar commented Mar 24, 2022

Yeah, while historical trace helps with troubleshooting and was immensly helpful during early development of lighthouse, it breaks the condition checks. For historical trace, it is better done as different types, like we do with addon.

vthapar added a commit to vthapar/lighthouse that referenced this issue Mar 24, 2022
Condition types are expected to be singelton and only last
state of a given condition type shoud be maintained. We currently
maintain a list of all transitions to a state as a list with max of
10. This causes problems with condition check APIs. Currently
support just one type, Valid.

Fixes submariner-io#714

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
@tpantelis tpantelis self-assigned this Mar 24, 2022
@tpantelis tpantelis removed this from Backlog in Backlog Mar 24, 2022
@tpantelis tpantelis added this to To do in Submariner Project 0.13 via automation Mar 24, 2022
@tpantelis
Copy link
Contributor

Yeah, while historical trace helps with troubleshooting and was immensly helpful during early development of lighthouse, it breaks the condition checks. For historical trace, it is better done as different types, like we do with addon.

Yup. I'll take care of this for 0.13.

Submariner Project 0.13 automation moved this from To do to Done Mar 28, 2022
sridhargaddam pushed a commit that referenced this issue Mar 28, 2022
Condition types are expected to be singelton and only last
state of a given condition type shoud be maintained. We currently
maintain a list of all transitions to a state as a list with max of
10. This causes problems with condition check APIs. Currently
support just one type, Valid.

Fixes #714

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/lighthouse that referenced this issue Jun 14, 2022
Condition types are expected to be singelton and only last
state of a given condition type shoud be maintained. We currently
maintain a list of all transitions to a state as a list with max of
10. This causes problems with condition check APIs. Currently
support just one type, Valid.

Fixes submariner-io#714

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
skitt pushed a commit that referenced this issue Jun 14, 2022
Condition types are expected to be singelton and only last
state of a given condition type shoud be maintained. We currently
maintain a list of all transitions to a state as a list with max of
10. This causes problems with condition check APIs. Currently
support just one type, Valid.

Fixes #714

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size:small
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants