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

Create certification test MQTT as GA in pubsub #952

Closed
artursouza opened this issue Jun 17, 2021 · 15 comments · Fixed by #1420
Closed

Create certification test MQTT as GA in pubsub #952

artursouza opened this issue Jun 17, 2021 · 15 comments · Fixed by #1420
Assignees
Milestone

Comments

@artursouza
Copy link
Member

Describe the feature

Certify MQTT as GA in pubsub

Release Note

RELEASE NOTE:

@rabollin
Copy link

Certify MQTT pubsub with 2 or 3 messaging services runs on MQTT protocol
It should handle queues, topics & subscriptions
Add Conformance tests covering all the specifications of the component

@rabollin
Copy link

components-contrib/.github/workflows/conformance.yml - Confirmance test for MQTT is already in place, need to create integration tests.

@artursouza artursouza modified the milestones: v1.5, v1.6 Oct 26, 2021
@shivamkm07
Copy link
Contributor

/assign

@shivamkm07
Copy link
Contributor

shivamkm07 commented Dec 14, 2021

MQTT certifcation testing

This project aims to test the MQTT Pub/Sub component under various conditions.

Test plan

Basic Test

  • Bring up a MQTT cluster
  • Start 1 sidecar/application(App1)
    • Publishes 1000+ unique messages
    • App: Simulate periodic errors
    • Component: Retries on error
    • App: Observes successful messages
    • Test: Confirms that all expected messages were received

Multiple Publishers-Subscribers

  • Start second sidecar/application(App2)
    • Each of the publishers publish a fixed number of messages to the topic
    • Test: Confirms that both applications receive all published messages

Infra Test

  • Start a constant flow of publishing and subscribing(App1)
    • Test: Keeps count of total sent/received
  • Start another sidecar/application with persistent session(App2)
    • Test: Publishes messages in background
    • Each of the applications should receive messages
  • Stop consumer connected with persistent session(App2)
    • Test: Publishes messages in background
    • Only App1 should receive messages
  • Stop publisher as well so that none of the components are active
    • No messages are published and received
  • Restart second consumer with persistent session
    • App2 receives all lost messages
  • Restart publisher so that both components are active
    • Test: Confirms that both applications received all published messages and no messages were lost

Network Test

  • Simulate network interruption
    • Test: Begins trying to reconnect & publish
    • Component: Begins trying to reconnect & re-subscribe

@shivamkm07
Copy link
Contributor

MQTT messages are still always Acked even if handler returns an error. But it seemingly requires change in paho.mqtt.golang lib and there already is an issue opened potentially addressing this.

See #1234 for more details.

@mukundansundar
Copy link
Contributor

The certification plan looks good to me.

MQTT messages are still always Acked even if handler returns an error. But it seemingly requires change in paho.mqtt.golang lib and there already is an issue opened potentially addressing this.

See #1234 for more details.

With this in mind ... I am not comfortable with certifying this as Stable without a fix or a workaround for this as this basically breaks Dapr PubSub API conformance at that point.

@artursouza
Copy link
Member Author

Please, add a new network scenario where the publisher is also validated that can recover from a reconnection.

@artursouza
Copy link
Member Author

MQTT messages are still always Acked even if handler returns an error. But it seemingly requires change in paho.mqtt.golang lib and there already is an issue opened potentially addressing this.

See #1234 for more details.

Can this be addressed in the dependency too? I am OK if we cut a PR against the dependency's repository.

@shivamkm07
Copy link
Contributor

Please, add a new network scenario where the publisher is also validated that can recover from a reconnection.

Updated test plan accordingly

@XavierGeerinck
Copy link

XavierGeerinck commented Dec 17, 2021

Question: will MQTT v5 be supported? According to the implementation (

func (m *mqttPubSub) createClientOptions(uri *url.URL, clientID string) *mqtt.ClientOptions {
) Dapr uses https://github.com/eclipse/paho.mqtt.golang while as this repository states that for MQTT v5 this library should be used: https://github.com/eclipse/paho.golang

P.S. MQTT v5 is needed for Azure IoT Hub Broker MQTT v5 support :) (@artursouza) main issue here is that username:password is not used but it requires SAS token setting... (more info: https://docs.microsoft.com/en-us/azure/iot-hub/iot-hub-mqtt-5#connection-lifecycle & https://docs.microsoft.com/en-us/azure/iot-hub/iot-hub-mqtt-5#authentication)

@yaron2
Copy link
Member

yaron2 commented Dec 17, 2021

Question: will MQTT v5 be supported? According to the implementation (

func (m *mqttPubSub) createClientOptions(uri *url.URL, clientID string) *mqtt.ClientOptions {

) Dapr uses https://github.com/eclipse/paho.mqtt.golang while as this repository states that for MQTT v5 this library should be used: https://github.com/eclipse/paho.golang
P.S. MQTT v5 is needed for Azure IoT Hub Broker MQTT v5 support :) (@artursouza) main issue here is that username:password is not used but it requires SAS token setting... (more info: https://docs.microsoft.com/en-us/azure/iot-hub/iot-hub-mqtt-5#connection-lifecycle & https://docs.microsoft.com/en-us/azure/iot-hub/iot-hub-mqtt-5#authentication)

We should make sure v5 is used, based on what I've seen.

@rabollin
Copy link

Question: will MQTT v5 be supported? According to the implementation (

func (m *mqttPubSub) createClientOptions(uri *url.URL, clientID string) *mqtt.ClientOptions {

) Dapr uses https://github.com/eclipse/paho.mqtt.golang while as this repository states that for MQTT v5 this library should be used: https://github.com/eclipse/paho.golang
P.S. MQTT v5 is needed for Azure IoT Hub Broker MQTT v5 support :) (@artursouza) main issue here is that username:password is not used but it requires SAS token setting... (more info: https://docs.microsoft.com/en-us/azure/iot-hub/iot-hub-mqtt-5#connection-lifecycle & https://docs.microsoft.com/en-us/azure/iot-hub/iot-hub-mqtt-5#authentication)

We should make sure v5 is used, based on what I've seen.

MQTT v5 support is being assessed and in consideration as it has multiple enhancements over the current version.

@shivamkm07
Copy link
Contributor

@XavierGeerinck @yaron2 MQTT v5 is still in beta release, so it can't be used till it gets to stable.
As per latest discussion with @artursouza , the plan is to get the current mqtt component to GA using MQTT3 and once MQTT v5 gets to stable, we can either upgrade thereafter or have a different component supporting v5. Let me know if it's good.

@XavierGeerinck
Copy link

Azure IoT Hub is going with MQTT v5 as well so I think Dapr should follow. But I would check this off with the Microsoft IoT Hub team? in the end they will benefit from this.

@shivamkm07
Copy link
Contributor

Seemingly MQTT doesn't guarantee ordering of messages(https://stackoverflow.com/questions/30955110/is-message-order-preserved-in-mqtt-messages) and so removed order part from test plan. Also verified the same while running tests.

@artursouza artursouza changed the title Certify MQTT as GA in pubsub Create certification test MQTT as GA in pubsub Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants