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

Cover cmd-exclude-prefixes-k8s by integration tests #13

Closed
wants to merge 3 commits into from
Closed

Cover cmd-exclude-prefixes-k8s by integration tests #13

wants to merge 3 commits into from

Conversation

tigrulya-exe
Copy link
Member

Should fix #11

  • Added required collector daemonset, service account, cluster role and cluster role binding.
  • Added integration tests for cmd-exclude-prefixes-k8s

@tigrulya-exe tigrulya-exe force-pushed the excluded-prefixes-tests branch from 9b91207 to 9442a34 Compare October 2, 2020 06:51
@tigrulya-exe tigrulya-exe marked this pull request as ready for review October 2, 2020 12:37
denis-tingaikin
denis-tingaikin requested changes Oct 5, 2020
View reviewed changes
go.mod Outdated Show resolved Hide resolved
k8s/k8s.go Outdated Show resolved Hide resolved
test/excluded_prefixes_collector_test.go Outdated Show resolved Hide resolved
test/excluded_prefixes_collector_test.go Outdated Show resolved Hide resolved
test/excluded_prefixes_collector_test.go Outdated Show resolved Hide resolved
test/excluded_prefixes_collector_test.go Outdated Show resolved Hide resolved
test/excluded_prefixes_collector_test.go Outdated Show resolved Hide resolved
@tigrulya-exe tigrulya-exe force-pushed the excluded-prefixes-tests branch 2 times, most recently from 4c83193 to 25f8a34 Compare October 5, 2020 12:09
denis-tingaikin
denis-tingaikin requested changes Oct 6, 2020
View reviewed changes
Copy link
Member

@denis-tingaikin denis-tingaikin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks better, but need to add some simplifications.

cluster-config.yaml Show resolved Hide resolved
k8s/k8s.go Show resolved Hide resolved
test/excluded_prefixes_collector_test.go Outdated Show resolved Hide resolved
test/excluded_prefixes_collector_test.go Outdated Show resolved Hide resolved
@tigrulya-exe tigrulya-exe force-pushed the excluded-prefixes-tests branch 4 times, most recently from 0f02b2b to 90810d5 Compare October 15, 2020 08:02
denis-tingaikin
denis-tingaikin reviewed Oct 22, 2020
View reviewed changes
Copy link
Member

@denis-tingaikin denis-tingaikin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good only one moment for simplification #13 (comment)

@tigrulya-exe tigrulya-exe force-pushed the excluded-prefixes-tests branch 2 times, most recently from fcd5ef7 to 0131dfa Compare October 23, 2020 06:42
@tigrulya-exe tigrulya-exe force-pushed the excluded-prefixes-tests branch 3 times, most recently from 6071246 to f86fb3c Compare October 28, 2020 07:19
@tigrulya-exe tigrulya-exe force-pushed the excluded-prefixes-tests branch 4 times, most recently from dfe6933 to 5689cc0 Compare November 3, 2020 11:39
@tigrulya-exe tigrulya-exe force-pushed the excluded-prefixes-tests branch 2 times, most recently from 0633056 to 897c5fd Compare November 24, 2020 13:04
Tigran Manasyan added 2 commits November 25, 2020 10:53
Added excluded prefixes collector integration tests
699de8b 
Signed-off-by: Tigran Manasyan <tigran.manasyan@xored.com>
Conflicts were resolved
637af73 
Signed-off-by: Tigran Manasyan <tigran.manasyan@xored.com>
Alpine was replaced with nsmgr daemonset
235512a 
Signed-off-by: Tigran Manasyan <tigran.manasyan@xored.com>
@tigrulya-exe tigrulya-exe force-pushed the excluded-prefixes-tests branch from 897c5fd to 235512a Compare November 25, 2020 04:53
denis-tingaikin
denis-tingaikin reviewed Nov 25, 2020
View reviewed changes
integration_k8s_kind_test/excluded_prefixes_collector_test.go
}

func (et *ExcludedPrefixesSuite) setupNsmgr() {
et.Require().NoError(k8s.ApplyDaemonSet("../deployments/nsmgr.yaml", func(nsmgr *v1.DaemonSet) {
Copy link
Member

Choose a reason for hiding this comment

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

Lets just edit ../deployments/nsmgr.yaml

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't we need nsmgr with spire volume and volumeMounts in other places? And I think nsmgr's namespace shouldn't be excluded-prefixes-collector by default.

@denis-tingaikin denis-tingaikin mentioned this pull request Dec 8, 2020
Closed
@denis-tingaikin
Copy link
Member

Outdated

nsmbot pushed a commit that referenced this pull request Jan 31, 2021
Update go.mod and go.sum to latest version from networkservicemesh/in…
85b32ef 
…tegration-tests@main networkservicemesh/integration-tests#13

networkservicemesh/integration-tests PR link: networkservicemesh/integration-tests#13

networkservicemesh/integration-tests commit message:
commit e0cf9c295e6ce123290903d44c172387ac13bbac
Author: Ed Warnicke <hagbard@gmail.com>
Date:   Sun Jan 31 14:53:51 2021 -0600

    Merge pull request #13 from denis-tingaikin/add-update-deps-job

    feat: add update deps job for integration-tests

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
@nsmbot nsmbot mentioned this pull request Jan 31, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@denis-tingaikin denis-tingaikin denis-tingaikin requested changes

@edwarnicke edwarnicke Awaiting requested review from edwarnicke

@haiodo haiodo Awaiting requested review from haiodo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cover cmd-exclude-prefixes-k8s by integration tests
2 participants
@tigrulya-exe @denis-tingaikin