Old bindingkey is removed if binding key is changed in configuration #468

Navyatha-reddi · 2020-06-12T15:15:53Z

Applicable Issues

Description of the Change

If binding key is changed in configuration, the old bindingkey is removed from mb queue.

Alternate Designs

Benefits

Reduction in conflicts between the binding keys.

Possible Drawbacks

Sign-off

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: Navyatha Reddi navyatha.reddi@tcs.com, Durga vasaadi durga.vasaadi@tcs.com, Surya Kumari Kolli

…oved from mb queue

Christoffer-Cortes · 2020-06-15T06:23:04Z

There is an error with that new bean:

Caused by: org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'rmqHandler': Unsatisfied dependency expressed through field 'rabbitManagementTemplate'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'deleteBindingKeys' defined in class path resource [com/ericsson/ei/handlers/RmqHandler.class]: Bean instantiation via factory method failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [org.springframework.amqp.rabbit.core.RabbitManagementTemplate]: Circular reference involving containing bean 'rmqHandler' - consider declaring the factory method as static for independence from its containing instance. Factory method 'deleteBindingKeys' threw exception; nested exception is org.springframework.web.client.ResourceAccessException: I/O error on GET request for "http://localhost:15672/api/bindings/": Connect to localhost:15672 [localhost/127.0.0.1] failed: Connection refused (Connection refused); nested exception is org.apache.http.conn.HttpHostConnectException: Connect to localhost:15672 [localhost/127.0.0.1] failed: Connection refused (Connection refused)

Christoffer-Cortes · 2020-06-15T10:26:48Z

src/main/java/com/ericsson/ei/handlers/RmqHandler.java


+    @Bean
+    public RabbitManagementTemplate deleteBindingKeys() {
+    	Integer rabbitPort=port;


Why not use primitive type int?

Christoffer-Cortes · 2020-06-15T10:32:09Z

src/main/java/com/ericsson/ei/handlers/RmqHandler.java

+    public RabbitManagementTemplate deleteBindingKeys() {
+    	Integer rabbitPort=port;
+    	String rabbitHttp = "https://";
+    	if(port==5672 && host.equals("localhost")){


What is the logic behind this code block? The API is exposed through the management page usually located at 15672 but if the port is anything other than 5672 we don't attempt to redirect to the website API? Won't this fail unless the user specifially configures the API url?

Hi,Actually for removing old binding keys we have gone through some websites on google, and found out the RabbitManagementTemplate to remove the old binding keys and used for implementation. So as part of it we are facing issues with RabbitManagementTemplate in test class. So we tried to solve it with this kind of approach, just for testing purpose.

tobiasake · 2020-06-16T06:58:11Z

src/main/java/com/ericsson/ei/handlers/RmqHandler.java

+    	if(port==5672){
+    		rabbitPort=15672;
+    	}
+        rabbitManagementTemplate = new RabbitManagementTemplate("http://"+host+":"+rabbitPort+"/api/",user,password);


What happens if un-secure http is disabled and only https is available in target environment?
Then this implementation will not work, I guess.

How is other Eiffel component handling restart of Eiffel components with new bindingKey ?

This binding change will happen from some of the Eiffel components, but rebind is not implemented in Eiffel components.

raja-maragani · 2020-06-16T13:02:25Z

src/main/java/com/ericsson/ei/handlers/RmqHandler.java

+    	if(port==5672){
+    		rabbitPort=15672;
+    	}
+        rabbitManagementTemplate = new RabbitManagementTemplate("http://"+host+":"+rabbitPort+"/api/",user,password);


RabbitManagementTemplate will use the http calls to connect with RabbitMQ and RabbitTemplate uses the amqp. both implementations are different. so better to change the new implementation to amqp based.

As per raja's comment, we have investigated for the other way of getting the binding keys form the RabbitMq,but rather that RabbitManagmentTemplate/Client we couldn't find any way to fetch the list of bindings using current EI implementation (Spring AMQP). Also digged into the code level referring the RabbitTemplate, channels and connectionFactory but there is no such functionality to get the bindingKeys.

So other way of approach for this implementation is using files, i.e to store the binding keys into the file/List and check with the existing bindingkeys, add or update the file with the required binding keys on EI start. The file gets updated when the binding configuration is changed.With this approach the manually created bindings cannot be retrieved which in turn cannot be deleted from the code.
Please suggest if this can be implemented in any other way.

How is other Eiffel component handling restart of Eiffel components with new bindingKey ?

Can't EI create a new queue with the new BindingKey name as suffix to the queue name and let the administrator choose to do with old queue and old event data, if it should be kept or removed.
EI starts to consume messages from new queue with new binding key(s) when started with new binding keys.
By doing that we don't wipe any persistent data or what it can cause.

Or if the administrator want the old queue and old bindingkeys to be removed automatically, set durable queue flag to False and old queue+bindingkeys will be removed automatically at shutdown and when EI starts a new queue will be created with new bindingkey(s), if I understand it correctly.

I think the only solution is to bind and unbind the queue when EI starts and shutdowns.
Since EI is not aware of previous binding keys used in other/previous running EI instances.
Documentation
https://pika.readthedocs.io/en/latest/modules/channel.html?highlight=bind#pika.channel.Channel.queue_unbind

If I understand what you trying to achieve.

I thinks its not safe/secure to let next EI instance to figure out what previously binding key was used and remove it.

Other option is what I commented about in previous comment, create a new queue with the new binding key and let rabbitmq remove the old queue+bindingkey with Queue TTL flag.

These two options is what I can find currently, but maybe it exist more options.

Had a discussion regarding the implementation way and the proposed solution suggested in the meeting is to use mongodb collection to store the binding keys and use the collection for removing the binding keys and updating the new binding keys.
Will start the implementation using mongo db with collection name "BindingKeys".

If we see this behavior on latest version of Eiffel Intelligence (and I think it is the same) we will need to transfer this fix also to the master branch, which will be included in an upcoming 4.x release (when the API versioning has been added)

Isn't "binding-keys" better collection name,, then its easier to administrate MongoDb if using mongo client command and typing the collection name in mongo shell.
Its easier to type only lower letters in the shell, instead typing mix of lower and upper letters

This collection need to be unique also for each EI instance if same mongodb is used, so collection name need to be configurable from application.config.
So we get unique collection name per instance, if same MongoDb is used.
Same MongoDb is used by tests and also in Easy2Use, maybe in other setups as well.

tobiasake

And tests is missing.

src/main/java/com/ericsson/ei/handlers/RmqHandler.java

…anges.

Navyatha-reddi · 2020-07-14T13:33:19Z

And tests is missing.

Tried implementing the test scenario for the binding keys stored in mongodb. Please help me out in any changes or any test cases to be added.

tobiasake · 2020-07-15T12:44:23Z

And tests is missing.

Tried implementing the test scenario for the binding keys stored in mongodb. Please help me out in any changes or any test cases to be added.
I see that you are using some RabbitAdmin library,, I guess that is a RabbitMq API specific library, does that work with normal AMQP standards and Qpid which is the embedded AMQP server that is used for functional tests.
If RabbitAdmin only works with RabbitMq and not AMQP in general, then I guess this is only possible to test as an integrationTests. I think we have the integrationTests in the EI-Frontend, which tests both backend and frontend .. ?
Someone can correct me if I am wrong!
From that test framework we use docker-compose and RabbitMq AMQP server.
In that test framework we could add integration test which tests start EI-backend with one bindingkey and then restart EI-Backend with new bindingkey and check RabbitMq queue if bindinkey has changed and also check that MongoDB bindingKey collection has removed the old bindingKey and newBindingkey has been added.
What is your ideas about testing this? @e-pettersson-ericsson @Christoffer-Cortes

Christoffer-Cortes · 2020-07-15T13:50:50Z

And tests is missing.

Tried implementing the test scenario for the binding keys stored in mongodb. Please help me out in any changes or any test cases to be added.
I see that you are using some RabbitAdmin library,, I guess that is a RabbitMq API specific library, does that work with normal AMQP standards and Qpid which is the embedded AMQP server that is used for functional tests.
If RabbitAdmin only works with RabbitMq and not AMQP in general, then I guess this is only possible to test as an integrationTests. I think we have the integrationTests in the EI-Frontend, which tests both backend and frontend .. ?
Someone can correct me if I am wrong!
From that test framework we use docker-compose and RabbitMq AMQP server.
In that test framework we could add integration test which tests start EI-backend with one bindingkey and then restart EI-Backend with new bindingkey and check RabbitMq queue if bindinkey has changed and also check that MongoDB bindingKey collection has removed the old bindingKey and newBindingkey has been added.
What is your ideas about testing this? @e-pettersson-ericsson @Christoffer-Cortes

Christoffer-Cortes · 2020-07-15T13:54:31Z

Sorry about the closing. =P Github reply/comment can be a bit confusing at times.
Anyway, either an integration test with docker components in the frontend if there is a way to programmatically restart a docker component... or a functionaltest and do like in scaling and failover test and start/close spring application from the code.

tobiasake

I will not be available coming weeks, so I will have no possibility to review coming changes on this PR.
So I set this to Approve for removing my "Request Changes" state on the PR.
If others review and Approve the change, then its ok to merge.

SantoshNC68 · 2020-07-23T06:05:43Z

Merging the PR.

If binding key is changed in configuration, the old bindingkey is rem…

614d983

…oved from mb queue

Navyatha-reddi force-pushed the 2.0.0-maintenance branch 5 times, most recently from b3be3d6 to 9ee48ff Compare June 13, 2020 10:37

Added Bean to the method

14c637e

Navyatha-reddi force-pushed the 2.0.0-maintenance branch from 9ee48ff to 14c637e Compare June 13, 2020 13:53

Christoffer-Cortes reviewed Jun 15, 2020

View reviewed changes

Added rabbitmq configurations to rabbitManagmentTemplate.

e2580ef

Navyatha-reddi force-pushed the 2.0.0-maintenance branch 3 times, most recently from 19f7964 to 0571e54 Compare June 16, 2020 06:13

Added code changes for test cases.

b2f72a0

Navyatha-reddi force-pushed the 2.0.0-maintenance branch from 0571e54 to b2f72a0 Compare June 16, 2020 06:46

tobiasake reviewed Jun 16, 2020

View reviewed changes

Added required changes for the failing testcases.

a4d5ffd

raja-maragani suggested changes Jun 16, 2020

View reviewed changes

Navyatha Reddi added 2 commits July 6, 2020 18:45

Implementation to store the binding keys using mongoDB.

438fcc0

Implemented with the new changes for testing the existing testcases.

ed6e930

tobiasake requested changes Jul 14, 2020

View reviewed changes

Navyatha Reddi added 4 commits July 14, 2020 16:55

Created collection name for binding keys and implemented requested ch…

47ff6ad

…anges.

Deleted unnecessary file.

3c88640

Testing the existing tests for the implemented changes.

1cf4901

Implemented testcase scenario for the changes.

eb96472

Added a new property for storing bindingkeys.

22f67f8

Christoffer-Cortes closed this Jul 15, 2020

Christoffer-Cortes reopened this Jul 15, 2020

Navyatha Reddi added 5 commits July 16, 2020 12:24

Added unit and functional testcases.

a89e576

Modifications in the testcase.

77ae69b

added changes for testcase.

3565f61

modified testcase.

fd15993

Added changes for the testcase.

e6d96f4

tobiasake approved these changes Jul 16, 2020

View reviewed changes

changed exchange in tests.

80d62d2

SantoshNC68 approved these changes Jul 17, 2020

View reviewed changes

Navyatha Reddi added 5 commits July 17, 2020 15:02

Added documentation.

7222fdb

modified documentation.

58bc47c

changed testcases.

4083691

removed tabs.

2aacded

uplifted version in pom.xml

077193b

raja-maragani approved these changes Jul 21, 2020

View reviewed changes

SantoshNC68 merged commit c373473 into eiffel-community:2.0.0-maintenance Jul 23, 2020

Old bindingkey is removed if binding key is changed in configuration #468

Old bindingkey is removed if binding key is changed in configuration #468

Uh oh!

Conversation

Navyatha-reddi commented Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Applicable Issues

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

Sign-off

Uh oh!

Christoffer-Cortes commented Jun 15, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasake Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasake Jul 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasake left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Navyatha-reddi commented Jul 14, 2020

Uh oh!

tobiasake commented Jul 15, 2020

Uh oh!

Christoffer-Cortes commented Jul 15, 2020

Uh oh!

Christoffer-Cortes commented Jul 15, 2020

Uh oh!

tobiasake left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SantoshNC68 commented Jul 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Navyatha-reddi commented Jun 12, 2020 •

edited

Loading

tobiasake Jun 23, 2020 •

edited

Loading

tobiasake Jul 7, 2020 •

edited

Loading

tobiasake left a comment •

edited

Loading