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

ncm-openstack: add ceilometer service #1383

Merged
merged 6 commits into from
Dec 5, 2019
Merged

Conversation

alvarosimon
Copy link
Member

NOTE: This PR requires #1354 to work, #1354 should be reviewed/merged first.

@jrha jrha added this to the 19.6 milestone Jun 3, 2019
@alvarosimon alvarosimon changed the title [WIP] ncm-openstack: add ceilometer service ncm-openstack: add ceilometer service Jul 16, 2019
# db_version is slow when not initialised
# (lots of retries before it gives up; can take up to 90s)
if ($self->_do([$self->{manage}, @{$self->{db_version}}], 'determine database version', test => 1)) {
if ($self->_do(['/bin/bash', '-c', join(' ', $self->{manage}, @{$self->{db_version}})], 'determine database version', test => 1)) {
Copy link
Member Author

@alvarosimon alvarosimon Aug 2, 2019

Choose a reason for hiding this comment

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

@stdweird this is just a workaround to avoid the execution issue with ceilometer-upgrade.
I have changed the unittests as well but now it is a bit messy, some commands are using /bin/bash workaround some of them are not.
We should provide a better solution (maybe just adding a new flag into CAF::Process to use the "right" environment?)

Copy link
Member

Choose a reason for hiding this comment

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

I've created a CAF issue for this, could you fill it in with more details of what would be required to avoid this workaround?
quattor/CAF#272

$self->verbose("Found existing db_version");
return 1 if ($self->{flavour} eq "rabbitmq");
} else {
$self->verbose("Database not yet available for service $self->{flavour}");
};

# Always populate/sync the databases
$self->pre_populate_service_database();
if ($self->_do([$self->{manage}, @{$self->{db_sync}}], 'populate database')) {
if ($self->_do(['/bin/bash', '-c', join(' ', $self->{manage}, @{$self->{db_sync}})], 'populate database')) {
Copy link
Member Author

Choose a reason for hiding this comment

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

same env issue with ceilometer-upgrade execution

@apdibbo
Copy link
Contributor

apdibbo commented Dec 5, 2019

This looks good to me on the proviso that I havent really checked the perl

apdibbo
apdibbo previously approved these changes Dec 5, 2019
Copy link
Contributor

@apdibbo apdibbo left a comment

Choose a reason for hiding this comment

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

This looks good to me

@jrha
Copy link
Member

jrha commented Dec 5, 2019

Rebased on master to avoid conflicts now that #1354 has been merged.

Copy link
Contributor

@apdibbo apdibbo left a comment

Choose a reason for hiding this comment

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

Still LGTM

@jrha jrha merged commit bea92e7 into quattor:master Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ncm-openstack: add ceilometer service support
3 participants