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

Added keyword for role create and delete #97

Open
wants to merge 50 commits into
base: master
Choose a base branch
from

Conversation

rohini-nubolab
Copy link

Added keyword for role create and delete

Fixes #96

.coveragerc Outdated
@@ -3,4 +3,4 @@ command_line = -m unittest discover
source =
src/
[report]
fail_under = 84
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't decrease coverage

Copy link
Author

Choose a reason for hiding this comment

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

done

@@ -1013,3 +1013,35 @@ def filter_endpoints_names(self, endpoints):
List of endpoint objects
"""
return [endpoints.metadata.name for endpoints in endpoints.items]

def get_role_details_in_namespace(self, name, namespace):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Name the functions same as Kubernetes python library function - in this case read_namespaced_role (see #75)

Copy link
Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still you need to do it for create_role_in_namespace and delete_role_in_namespace

@m-wcislo
Copy link
Collaborator

Complete checklist

At least one example testcase added in testcases/
Library Documentation regenerated according to Generate docs
All new testcases tagged as prerelease along other tags to exclude it from execution until released on PyPI
Coverage threshold increased in .coveragerc if new coverage is higher than actual, see the lint-and-coverage step in CI

@rohini-nubolab
Copy link
Author

Complete checklist

At least one example testcase added in testcases/
Library Documentation regenerated according to Generate docs
All new testcases tagged as prerelease along other tags to exclude it from execution until released on PyPI
Coverage threshold increased in .coveragerc if new coverage is higher than actual, see the lint-and-coverage step in CI

done

mock_lnp.side_effect = mock_create_role_in_namespace
kl = KubeLibrary(kube_config='test/resources/k3d')
name = 'pod-reader'
role_manifest = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this test has no value, we are not testing anything as everything is mocked. One thing that might be useful is testing handling situation when role is already created with some meaningful output.

@@ -678,3 +694,53 @@ def test_get_cron_job_details_in_namespace(self, mock_lnp):
kl = KubeLibrary(kube_config='test/resources/k3d')
cron_job_details = kl.get_cron_job_details_in_namespace('hello', 'default')
self.assertEqual('mytestlabel', cron_job_details.items.metadata.labels.TestLabel)

@mock.patch('kubernetes.client.RbacAuthorizationV1Api.delete_namespaced_role')
def test_delete_role_in_namespace(self, mock_lnp):
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could test handling situation when there is no role to remove. Other than that the mocked response is wrong because as I understand it will return V1 status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add keyword for create and delete role ,role binding
2 participants