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

Add support for NetworkPolicies #422

Merged
merged 1 commit into from
Feb 28, 2019
Merged

Conversation

n1koo
Copy link
Contributor

@n1koo n1koo commented Feb 22, 2019

What are you trying to accomplish with this PR?
Add support for NetworkPolicy so that deploys dont nag about it

How is this accomplished?
Add resource type, copypaste things

What could go wrong?
I added support for pruning too because thats the correct way to do it, but might suprise people that do NetworkPolicies outside of deploys. Hopefully nobody does such a thing :)

@n1koo n1koo force-pushed the dont_nag_on_netpols branch 2 times, most recently from 59e4703 to 3a48ee0 Compare February 22, 2019 12:02
Copy link
Contributor

@dturn dturn left a comment

Choose a reason for hiding this comment

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

Seems reasonable given this resource has no status. Though I wonder if we want to use the UNUSUAL_FAILURE_MESSAGE instead of STANDARD_TIMEOUT_MESSAGE for timeout_message.

@n1koo
Copy link
Contributor Author

n1koo commented Feb 23, 2019

Yeah that makes sense, changed

@dturn dturn requested a review from KnVerey February 25, 2019 16:15
@@ -1080,6 +1081,24 @@ def test_not_apply_resource_can_be_pruned
])
end

def test_network_policies_are_deployed_first
assert_deploy_success(deploy_fixtures("network-policy"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating a separate fixture set with just this in it, on this line you can do deploy_fixtures('hello-cloud', subset: ['network_policy.yml'])

"NetworkPolicy/allow-all-network-policy",
], in_order: true)

netpols = networking_v1_kubeclient.get_network_policies(namespace: @namespace)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using kubeclient here, please augment the HelloCloud FixtureSet as described in our Readme:

Add the a basic example of the type to the hello-cloud fixture set and appropriate assertions to #assert_all_up in hello_cloud.rb. This will get you coverage in several existing tests, such as test_full_hello_cloud_set_deploy_succeeds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right missed that, sorry. Updated now

@n1koo n1koo force-pushed the dont_nag_on_netpols branch 2 times, most recently from 9588e85 to ec887ea Compare February 27, 2019 06:21
@dturn dturn merged commit f0ee52a into Shopify:master Feb 28, 2019
@timothysmith0609 timothysmith0609 temporarily deployed to rubygems March 27, 2019 16:53 Inactive
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.

None yet

4 participants