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.

Sign up for GitHub

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

Jump to bottom

Integration tests for Openstack #154

Merged
merged 2 commits into from
Oct 23, 2020
Merged

Integration tests for Openstack #154

merged 2 commits into from
Oct 23, 2020

Conversation

prashanth26
Copy link

@prashanth26 prashanth26 commented Oct 5, 2020 •
edited

Co-authored-by: DockToFuture sebastian.stauch@sap.com

How to categorize this PR?

/area dev-productivity
/kind test
/priority normal
/platform openstack

What this PR does / why we need it:
Added integration tests for the infrastructure controller.

Which issue(s) this PR fixes:
Fixes #87

Special notes for your reviewer:
The following PR covers the basic cases for integration tests for OpenStack. Any review is appreciated.

Release note:

A new integration test for infrastructure creation and deletion has been added.

@prashanth26 prashanth26 requested review from a team as code owners October 5, 2020 10:52
@gardener-robot gardener-robot added kind/test Test platform/openstack OpenStack platform/infrastructure priority/normal labels Oct 5, 2020
@gardener-robot
Copy link

@prashanth26 Label area/testing does not exist.

@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 5, 2020
@gardener-robot gardener-robot added the area/dev-productivity Developer productivity related (how to improve development) label Oct 5, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 5, 2020
dkistner
dkistner reviewed Oct 10, 2020
View reviewed changes
Copy link
Member

@dkistner dkistner left a comment

Choose a reason for hiding this comment

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

Awesome PR. Thanks a lot.

Tried it out, working well.
I have just a few comments and suggestions :)

test/integration/infrastructure/infrastructure_test.go Outdated Show resolved Hide resolved
test/integration/infrastructure/infrastructure_test.go Outdated Show resolved Hide resolved
test/integration/infrastructure/infrastructure_test.go Outdated Show resolved Hide resolved
test/integration/infrastructure/infrastructure_test.go Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added needs/rebase Needs git rebase size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Oct 20, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 20, 2020
@prashanth26 @DockToFuture
Integration tests for Openstack
9517696 
Co-authored-by: DockToFuture <sebastian.stauch@sap.com>
dkistner
dkistner previously approved these changes Oct 20, 2020
View reviewed changes
Copy link
Member

@dkistner dkistner left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/rebase Needs git rebase needs/second-opinion Needs second review by someone else labels Oct 20, 2020
@dkistner
Copy link
Member

/invite @kon-angelo

PR needs rebase

@gardener-robot gardener-robot added needs/review Needs review and removed reviewed/lgtm Has approval for merging needs/review Needs review labels Oct 20, 2020
@gardener-robot gardener-robot added the needs/second-opinion Needs second review by someone else label Oct 20, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 20, 2020
@rfranzke
Copy link
Member

/status author-action

@gardener-robot gardener-robot added the status/author-action Issue requires issue author action label Oct 21, 2020
@gardener-robot
Copy link

@prashanth26 The pull request was assigned to you under author-action. Please unassign yourself when you are done. Thank you.

@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 22, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 22, 2020
@prashanth26
Copy link
Author

Thanks, @DockToFuture for addressing the review comments :)

@DockToFuture
Copy link
Member

I've addressed the issues from @kon-angelo accordingly and updated the PR.
Currently the floating pool name is missing in the shoot-operator-ccee-eu-nl-1 secret, @dguendisch wants to add it.

@dguendisch
Copy link
Member

The floating pool name is now available as secret key floatingPoolName.

kon-angelo
kon-angelo approved these changes Oct 22, 2020
View reviewed changes
Copy link
Contributor

@kon-angelo kon-angelo left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/second-opinion Needs second review by someone else status/author-action Issue requires issue author action labels Oct 22, 2020
@dkistner dkistner merged commit 44d5d99 into gardener:master Oct 23, 2020
@timebertt
Copy link
Member

/test-single

@testmachinery
Copy link

testmachinery bot commented Nov 21, 2020 •
edited

Testrun: e2e-xjnkx
Workflow: e2e-xjnkx-wf
Phase: Failed

+---------------------+---------------------+--------+----------+
|        NAME         |        STEP         | PHASE  | DURATION |
+---------------------+---------------------+--------+----------+
| infrastructure-test | infrastructure-test | Failed | 2m52s    |
+---------------------+---------------------+--------+----------+

@timebertt
Copy link
Member

The TestDefinition is erroneous:

2020-11-21 15:41:32 | /src/test/integration/infrastructure/infrastructure_test.go:74
2020-11-21 15:41:32 | --domain-name flag is not specified
2020-11-21 15:41:32 | Test Panicked

The TestDefinition itself is fixed with #179

@gardener-robot gardener-robot added priority/3 Priority (lower number equals higher priority) and removed priority/3 Priority (lower number equals higher priority) labels Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkistner dkistner dkistner approved these changes

@kon-angelo kon-angelo kon-angelo approved these changes

Assignees

@prashanth26 prashanth26

Labels
area/dev-productivity Developer productivity related (how to improve development) kind/test Test needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/openstack OpenStack platform/infrastructure reviewed/lgtm Has approval for merging size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add integration test for Infrastructure reconciliation and deletion