search

yandex-qatools / teamcity-openstack-plugin

Teamcity plugin to add openstack integration
Other
25 stars 24 forks source link

fix instance removing while profile changes #48

Closed promiteyka closed 4 years ago

promiteyka commented 4 years ago

Fix for https://github.com/yandex-qatools/teamcity-openstack-plugin/issues/2

axel3rd commented 4 years ago

@promiteyka : Many thanks for this pull request. Could the SonarQube quality gate taken into account ?

About unit tests, are they successful locally or not ?

promiteyka commented 4 years ago

@promiteyka : Many thanks for this pull request. Could the SonarQube quality gate taken into account ?

About unit tests, are they successful locally or not ?

We did not change the tests, but we checked the modified plugin on our server and it works as expected. I propose to finalize the code as part of this pull request.

promiteyka commented 4 years ago

@axel3rd The first build - https://teamcity.jetbrains.com/viewLog.html?buildId=2975379&buildTypeId=TeamCityThirdPartyPlugins_OpenStackCloudSupport_PullRequestAnalysis the tests failed because of 500 errors on devstack the second build finished successfully https://teamcity.jetbrains.com/viewLog.html?buildId=2975435&buildTypeId=TeamCityThirdPartyPlugins_OpenStackCloudSupport_PullRequestAnalysis without any errors.

Now we are working with sonarqube errors.

promiteyka commented 4 years ago

We tested the updated plugin on versions 2019.2.4 and 2020.1. We installed it on production servers.

axel3rd commented 4 years ago

We tested the updated plugin on versions 2019.2.4 and 2020.1. We installed it on production servers.

@promiteyka: Thanks for work & tests

Some questions:

  1. Can you squash all PR commits in one (Comment sample: Fix #2 instance removing while profile changes) ? if I do it with GitHub option during merge/rebase, I don't know what it append with multiple-committers.
  2. The SonarQube quality gate is currently red due no (new) unit-tests (and not introducing new Code Smells would be great). To have sustainable plugin, is it relevant to achieve that ? I can rebase/merge without it, but I will not do the 1.3 release without take time to fix it.

Let me know for this 2 points.

For informations/check, executing unit test OpenstackCloudClientTest.testV3 on some Openstack platform where some existing VM in tenant and where the stop process is a little long, I can see:

Retrieve floating ip for future instance association
Floating ip: 10.0.0.1
Creating openstack instance with [removed]
Associating floating ip to serverId 360a0989-77f1-4237-b619-d08680249953
(Waiting fixed ip before floating ip association on serverId: 360a0989-77f1-4237-b619-d08680249953)
Restore instances of openstack image id=377f47ef-f656-4349-a135-0e72fce24c1e ...
Cloud instance restored: id=360a0989-77f1-4237-b619-d08680249953, name=openstack-test-teamcity-plugin-1592390169863, status=RUNNING
Pinging openstack-test-teamcity-plugin-1592390178908 for status
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390178908', result is 'ACTIVE' (previous internal status setted was 'Running')
Pinging openstack-test-teamcity-plugin-1592390169863 for status
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390169863', result is 'ACTIVE' (previous internal status setted was 'Starting')
2020-06-17 12:36:20 INFO  clouds.:91 - Stopping cloud openstack instance openstack-test-teamcity-plugin-1592390169863
Pinging openstack-test-teamcity-plugin-1592390178908 for status
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390178908', result is 'ACTIVE' (previous internal status setted was 'Running')
Pinging openstack-test-teamcity-plugin-1592390169863 for status
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390169863', result is 'ACTIVE' (previous internal status setted was 'Scheduled to stop')
Pinging openstack-test-teamcity-plugin-1592390178908 for status
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390178908', result is 'ACTIVE' (previous internal status setted was 'Running')
Pinging openstack-test-teamcity-plugin-1592390169863 for status
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390169863', result is 'ACTIVE' (previous internal status setted was 'Stopping')
Pinging openstack-test-teamcity-plugin-1592390178908 for status
# Multiple Running/Stopping phase
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390178908', result is 'ACTIVE' (previous internal status setted was 'Running')
Pinging openstack-test-teamcity-plugin-1592390169863 for status
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390169863', result is 'ACTIVE' (previous internal status setted was 'Stopping')
Pinging openstack-test-teamcity-plugin-1592390178908 for status
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390178908', result is 'SHUTOFF' (previous internal status setted was 'Running')
2020-06-17 12:37:23 INFO  clouds.:91 - Terminating cloud openstack instance openstack-test-teamcity-plugin-1592390178908
Pinging openstack-test-teamcity-plugin-1592390169863 for status
Getting instance status from openstack for 'openstack-test-teamcity-plugin-1592390169863', result is 'SHUTOFF' (previous internal status setted was 'Stopping')
2020-06-17 12:37:24 INFO  clouds.:91 - Terminating cloud openstack instance openstack-test-teamcity-plugin-1592390169863
2020-06-17 12:37:25 INFO  clouds.:91 - OpenstackCloudImage.dispose ...

The process pings 2 different instances, but only one is existing in Openstack platform. Don't know if the problem comes from Id usage or restore from existing VMs.

axel3rd commented 4 years ago

@promiteyka : Please see PR #1 on your repo about my previous comments.

promiteyka commented 4 years ago

About a squash commit, should I reopen a new pull request with all the changes in one commit?

axel3rd commented 4 years ago

About a squash commit, should I reopen a new pull request with all the changes in one commit?

No, you can do it directly on branch linked to pull request, by using a force push (after the squash).

If master is used (ie like the current: promiteyka:master), perhaps allowing force push on master branch could be required.

axel3rd commented 4 years ago

I have reintroduced a potential bug in my commit fefe6109d9f4c012bab10bcb3590f87d76cdf5f2 :confounded: :thumbsdown: :facepunch:

promiteyka commented 4 years ago

Squash commit is done.

promiteyka commented 4 years ago

I have reintroduced a potential bug in my commit fefe6109d9f4c012bab10bcb3590f87d76cdf5f2 😖 👎 👊

What do we do next?

axel3rd commented 4 years ago

Squash commit is done.

@promiteyka: Many thanks. A remark about this commit, the author is me due to my last PR (axel3rd authored and Dmitry.Vasilyev committed on 18 Feb). If we want you become contributor of this plugin with this commit, I think you must amend it (git commit --amend --author="John Doe <john@doe.org>", with correct name & email using for for GitHub pseudo).

What do we do next?

If you want take in attention my previous comment (the amend) ; you can quickly fix it (fix, squash 2 last commit ; it will fix the amend) and I would thank you ; otherwise I will fix it after the rebase/merge of this PR.

Let me know your preferences

axel3rd commented 4 years ago

Let me know your preferences

@promiteyka : No preferences (mainly about first comment) ? Merge I the PR "as it" 😢 ?

promiteyka commented 4 years ago

@axel3rd I apologize for the delay, was on a short vacation. It is not important for me to be a contributor; for me, just a working plugin is enough. Merge them please. Thank you.