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

Fixed rabbitmq upgrade #1989

Merged
merged 13 commits into from
Jan 26, 2021

Conversation

atsikham
Copy link
Contributor

@atsikham atsikham commented Jan 21, 2021

Tasks #1979 #1984. Made it in the one PR as fixes in the same place are necessary.
Tried to find universal way to check clustering and rabbitmq version.

@przemyslavic
Copy link
Collaborator

przemyslavic commented Jan 21, 2021

And what about running rabbitmq-diagnostics server_version command to check rabbitmq server version?

root@2334820499a8:/# rabbitmq-diagnostics server_version
Asking node rabbit@2334820499a8 for its RabbitMQ version...
3.8.9

root@fba4e177a16f:/# rabbitmq-diagnostics server_version
Asking node rabbit@fba4e177a16f for its RabbitMQ version...
3.7.10

@atsikham atsikham marked this pull request as draft January 22, 2021 06:22
@atsikham
Copy link
Contributor Author

Additional comment from @przemyslavic:

What about an upgrade that fails for some reason, but installs new packages before that. The next time the upgrade is started, it will be skipped, because the package version will be up-to-date (and this condition is checked). From what I saw in other components, >= conditions were used just in case the upgrade was interrupted.

@atsikham atsikham marked this pull request as ready for review January 22, 2021 07:08
@atsikham
Copy link
Contributor Author

And what about running rabbitmq-diagnostics server_version command to check rabbitmq server version?

root@2334820499a8:/# rabbitmq-diagnostics server_version
Asking node rabbit@2334820499a8 for its RabbitMQ version...
3.8.9

root@fba4e177a16f:/# rabbitmq-diagnostics server_version
Asking node rabbit@fba4e177a16f for its RabbitMQ version...
3.7.10

Used rabbitmqctl instead with json formatter as @sk4zuzu suggested.

@atsikham atsikham requested a review from sk4zuzu January 22, 2021 07:09
sk4zuzu
sk4zuzu previously approved these changes Jan 22, 2021
Copy link
Contributor

@sk4zuzu sk4zuzu left a comment

Choose a reason for hiding this comment

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

LGTM

przemyslavic
przemyslavic previously approved these changes Jan 22, 2021
@sk4zuzu
Copy link
Contributor

sk4zuzu commented Jan 22, 2021

@atsikham dont we need changelog entry here ? 🤔

@atsikham
Copy link
Contributor Author

atsikham commented Jan 22, 2021

@atsikham dont we need changelog entry here ? 🤔

As we had a release and refer to these bugs in the changelog as "known issues", I will update the changelog.

@przemyslavic can we test it somehow before merge?

@przemyslavic
Copy link
Collaborator

I'll try to test this PR today. Let's not merge it for now.

@przemyslavic
Copy link
Collaborator

/azp run

@hitachienergy hitachienergy deleted a comment from azure-pipelines bot Jan 22, 2021
@atsikham
Copy link
Contributor Author

Tested manually on aws for clustered and not clustered installations.

@przemyslavic
Copy link
Collaborator

@atsikham still doesn't work for v3.7.10:

2021-01-22T12:47:32.7905997Z[38;21m12:47:32 INFO cli.engine.ansible.AnsibleCommand - TASK [upgrade : RabbitMQ | Set facts] ******************************************[0m
2021-01-22T12:47:32.9181086Z[31;21m12:47:32 ERROR cli.engine.ansible.AnsibleCommand - fatal: [ec2-63-35-172-154.eu-west-1.compute.amazonaws.com]: FAILED! => {"msg": "The task includes an option with an undefined variable. The error was: 'dict object' has no attribute 'rabbitmq_version'\n\nThe error appears to be in '/shared/build/06todevawubcanal/ansible/roles/upgrade/tasks/rabbitmq.yml': line 17, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n- name: RabbitMQ | Set facts\n  ^ here\n"}[0m
2021-01-22T12:47:32.9344032Z[31;21m12:47:32 ERROR cli.engine.ansible.AnsibleCommand - fatal: [ec2-18-200-195-163.eu-west-1.compute.amazonaws.com]: FAILED! => {"msg": "The task includes an option with an undefined variable. The error was: 'dict object' has no attribute 'rabbitmq_version'\n\nThe error appears to be in '/shared/build/06todevawubcanal/ansible/roles/upgrade/tasks/rabbitmq.yml': line 17, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n- name: RabbitMQ | Set facts\n  ^ here\n"}

No rabbitmq_version key found in json output.

@atsikham
Copy link
Contributor Author

/azp run

@to-bar
Copy link
Contributor

to-bar commented Jan 22, 2021

How about using lock file approach (upgrade in progress flag file) instead of this condition:
when: rabbitmq_version_current is version(versions.general, '<=')

The flag file is used for example for ZooKeeper, IMO is a smart way 🏃🏻

@przemyslavic
Copy link
Collaborator

/azp run

@atsikham atsikham requested a review from sk4zuzu January 25, 2021 11:45
sk4zuzu
sk4zuzu previously approved these changes Jan 25, 2021
Copy link
Contributor

@sk4zuzu sk4zuzu left a comment

Choose a reason for hiding this comment

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

❤️

@atsikham
Copy link
Contributor Author

/azp run

@przemyslavic
Copy link
Collaborator

@atsikham tested and approved :)

@atsikham atsikham merged commit e53f983 into hitachienergy:develop Jan 26, 2021
@atsikham atsikham self-assigned this Jan 26, 2021
sbbroot pushed a commit to sbbroot/epiphany that referenced this pull request Aug 17, 2021
* Fixed rabbitmq upgrade

* Updates according to review

* Changed shell to command after latest changes

* Updated changelog

* Changed version check to supported by 3.7.x command

* Applied lock file approach for rabbitmq upgrade

* Updated task names

* Fixed a few linter warnings

Co-authored-by: atsikham <atsikham@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants