Skip to content

[CI-v2] Add loadbalancer job with Github Action#2328

Merged
mandre merged 1 commit intogophercloud:masterfrom
EmilienM:civ2_loadbalancer
Jan 28, 2022
Merged

[CI-v2] Add loadbalancer job with Github Action#2328
mandre merged 1 commit intogophercloud:masterfrom
EmilienM:civ2_loadbalancer

Conversation

@EmilienM
Copy link
Copy Markdown
Contributor

@EmilienM EmilienM commented Jan 21, 2022

  • Deploy Octavia with Devstack
  • Run loadbalancer acceptance tests
  • Introduce IsReleasesAbove and IsReleasesBelow which return True if a
    release is above the current branch (or below).
  • Only test tls_versions in Victoria and beyond, not supported before.
  • Only add l7 parameters to QuotaUpdate in Victoria and beyond, not
    supported before.
  • Remove all the skips
  • Logs failures if any

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 21, 2022

Coverage Status

Coverage remained the same at 79.91% when pulling 44bf8e8 on EmilienM:civ2_loadbalancer into 86aa60d on gophercloud:master.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jan 21, 2022

Build failed.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jan 21, 2022

Build failed.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jan 21, 2022

Build failed.

* Deploy Octavia with Devstack
* Run loadbalancer acceptance tests
* Introduce IsReleasesAbove and IsReleasesBelow which return True if a
  release is above the current branch (or below).
* Only test tls_versions in Victoria and beyond, not supported before.
* Only add l7 parameters to QuotaUpdate in Victoria and beyond, not
  supported before.
* Remove all the skips
* Logs failures if any
branch: ${{ matrix.openstack_version }}
conf_overrides: |
enable_plugin octavia https://opendev.org/openstack/octavia ${{ matrix.openstack_version }}
enable_plugin neutron https://opendev.org/openstack/neutron ${{ matrix.openstack_version }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't neutron part of core devstack? It feels a bit strange to have to enable it explicitly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we need the plugin from neutron because there are some functions called from devstack core, it's weird but it's like this (and it's only for some versions)... I think this is fine, upstream gate does the same.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

// IsReleasesAbove will return true on releases above a certain
// one. The result is always true on master release. Releases are named such
// as 'stable/mitaka', master, etc.
func IsReleasesAbove(t *testing.T, release string) bool {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should use these new functions in SkipReleasesAbove() and SkipReleasesBelow() to avoid duplicating the logic. Not necessarily for this patch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, got it. I'll prepare a patch soon.

}).Extract()
}
// L7 parameters are only supported in microversion v2.19 introduced in victoria
if clients.IsReleasesAbove(t, "stable/ussuri") {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unrelated to your patch but it looks like the implementation of L7 parameters is buggy, they should be pointers according to https://github.com/gophercloud/gophercloud/blob/master/docs/MICROVERSIONS.md#new-response-fields.

Same for tls version above.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

could you file an issue please?

@mandre
Copy link
Copy Markdown
Contributor

mandre commented Jan 28, 2022

Let's merge this. We can always refactor IsReleasesAbove() and friends in a follow-up patch, no urgency.

@mandre mandre merged commit 995923a into gophercloud:master Jan 28, 2022
@EmilienM EmilienM deleted the civ2_loadbalancer branch January 28, 2022 13:45
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.

3 participants