Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Delete defaultAllowIPSet on namespace delete #3250

Merged
merged 1 commit into from
Mar 8, 2018
Merged

Delete defaultAllowIPSet on namespace delete #3250

merged 1 commit into from
Mar 8, 2018

Conversation

alok87
Copy link
Contributor

@alok87 alok87 commented Mar 1, 2018

What ?

Fix for #3247

What it does?

On namespace deletion default ipset will cleaned for the namespace

@alok87 alok87 mentioned this pull request Mar 1, 2018
Closed
@alok87
Copy link
Contributor Author

alok87 commented Mar 1, 2018

@bboreham The code works, deployed in our staging cluster and tested the same, it works.

  • kubectl create ns aks
INFO: 2018/03/01 13:06:04.276064 EVENT AddNamespace {"metadata":{"creationTimestamp":"2018-03-01T13:06:04Z","name":"aks","resourceVersion":"44605956","selfLink":"/api/v1/namespaces/aks","uid":"4c32a517-1d51-11e8-900b-027f607c7b80"},"spec":{"finalizers":["kubernetes"]},"status":{"phase":"Active"}}
INFO: 2018/03/01 13:06:04.278721 creating ipset: &npc.selectorSpec{key:"", selector:labels.internalSelector{}, dst:false, ipsetType:"hash:ip", ipsetName:"weave-usb0iYnjHs@=O7u8u/9gTbt%b", nsName:"aks"}
DEBU: 2018/03/01 13:06:04.280479 ensuring rule for DefaultAllow in namespace: aks, set weave-#[w{=fp4BYyel2u|{f;WvV5Wr
  • kubectl delete ns aks
INFO: 2018/03/01 13:08:03.029812 EVENT UpdateNamespace {"metadata":{"creationTimestamp":"2018-03-01T13:06:04Z","name":"aks","resourceVersion":"44605956","selfLink":"/api/v1/namespaces/aks","uid":"4c32a517-1d51-11e8-900b-027f607c7b80"},"spec":{"finalizers":["kubernetes"]},"status":{"phase":"Active"}} {"metadata":{"creationTimestamp":"2018-03-01T13:06:04Z","deletionTimestamp":"2018-03-01T13:08:03Z","name":"aks","resourceVersion":"44606528","selfLink":"/api/v1/namespaces/aks","uid":"4c32a517-1d51-11e8-900b-027f607c7b80"},"spec":{"finalizers":["kubernetes"]},"status":{"phase":"Terminating"}}
DEBU: 2018/03/01 13:08:06.067467 EVENT UpdatePod {"metadata":{"annotations":{"kubernetes.io/created-by":"{\"kind\":\"SerializedReference\",\"apiVersion\":\"v1\",\"reference\":{\"kind\":\"ReplicaSet\",\"namespace\":\"revert\",\"name\":\"paisa-876bddb-2026825816\",\"uid\":\"e5c139ce-1d1e-11e8-ac8d-0238e90e49f8\",\"apiVersion\":\"extensions\",\"resourceVersion\":\"44498905\"}}\n"},"creationTimestamp":"2018-03-01T07:05:17Z","generateName":"paisa-876bddb-2026825816-","labels":{"app":"paisa","pod-template-hash":"2026825816","version":"876bddb-20180301-070513"},"name":"paisa-876bddb-2026825816-fl75r","namespace":"revert","resourceVersion":"44606474","selfLink":"/api/v1/namespaces/revert/pods/paisa-876bddb-2026825816-fl75r","uid":"e5d6d5fa-1d1e-11e8-ac8d-0238e90e49f8"},"spec":{"containers":[{"image":"practodev/paisa:staging-876bddb04dacaf649afd0c5a344ccd5b15f1ca64","imagePullPolicy":"IfNotPresent","name":"paisa-876bddb","ports":[{"containerPort":80,"protocol":"TCP"}],"terminationMessagePath":"/dev/termination-log","terminationMessagePolicy":"File"}],"dnsPolicy":"ClusterFirst","imagePullSecrets":[{"name":"dev-secret-docker"}],"nodeName":"ip-172-31-102-186.ap-south-1.compute.internal","restartPolicy":"Always","schedulerName":"default-scheduler","securityContext":{},"serviceAccount":"default","serviceAccountName":"default","terminationGracePeriodSeconds":30},"status":{"conditions":[{"lastProbeTime":null,"lastTransitionTime":"2018-03-01T07:05:48Z","status":"True","type":"Initialized"},{"lastProbeTime":null,"lastTransitionTime":"2018-03-01T13:07:51Z","message":"containers with unready status: [paisa-876bddb]","reason":"ContainersNotReady","status":"False","type":"Ready"},{"lastProbeTime":null,"lastTransitionTime":"2018-03-01T07:05:48Z","status":"True","type":"PodScheduled"}],"hostIP":"172.31.102.186","phase":"Running","podIP":"100.114.0.9","qosClass":"Burstable","startTime":"2018-03-01T07:05:48Z"}} {"metadata":{"annotations":{"kubernetes.io/created-by":"{\"kind\":\"SerializedReference\",\"apiVersion\":\"v1\",\"reference\":{\"kind\":\"ReplicaSet\",\"namespace\":\"revert\",\"name\":\"paisa-876bddb-2026825816\",\"uid\":\"e5c139ce-1d1e-11e8-ac8d-0238e90e49f8\",\"apiVersion\":\"extensions\",\"resourceVersion\":\"44498905\"}}\n"},"creationTimestamp":"2018-03-01T07:05:17Z","generateName":"paisa-876bddb-2026825816-","labels":{"app":"paisa","pod-template-hash":"2026825816","version":"876bddb-20180301-070513"},"name":"paisa-876bddb-2026825816-fl75r","namespace":"revert","resourceVersion":"44606541","selfLink":"/api/v1/namespaces/revert/pods/paisa-876bddb-2026825816-fl75r","uid":"e5d6d5fa-1d1e-11e8-ac8d-0238e90e49f8"},"spec":{"containers":[{"image":"practodev/paisa:staging-876bddb04dacaf649afd0c5a344ccd5b15f1ca64","imagePullPolicy":"IfNotPresent","name":"paisa-876bddb","ports":[{"containerPort":80,"protocol":"TCP"}],"terminationMessagePath":"/dev/termination-log","terminationMessagePolicy":"File"}],"dnsPolicy":"ClusterFirst","imagePullSecrets":[{"name":"dev-secret-docker"}],"nodeName":"ip-172-31-102-186.ap-south-1.compute.internal","restartPolicy":"Always","schedulerName":"default-scheduler","securityContext":{},"serviceAccount":"default","serviceAccountName":"default","terminationGracePeriodSeconds":30},"status":{"conditions":[{"lastProbeTime":null,"lastTransitionTime":"2018-03-01T07:05:48Z","status":"True","type":"Initialized"},{"lastProbeTime":null,"lastTransitionTime":"2018-03-01T13:07:51Z","message":"containers with unready status: [paisa-876bddb]","reason":"ContainersNotReady","status":"False","type":"Ready"},{"lastProbeTime":null,"lastTransitionTime":"2018-03-01T07:05:48Z","status":"True","type":"PodScheduled"}],"hostIP":"172.31.102.186","phase":"Running","podIP":"100.114.0.9","qosClass":"Burstable","startTime":"2018-03-01T07:05:48Z"}}
INFO: 2018/03/01 13:08:08.271573 EVENT UpdateNamespace {"metadata":{"creationTimestamp":"2018-03-01T13:06:04Z","deletionTimestamp":"2018-03-01T13:08:03Z","name":"aks","resourceVersion":"44606528","selfLink":"/api/v1/namespaces/aks","uid":"4c32a517-1d51-11e8-900b-027f607c7b80"},"spec":{"finalizers":["kubernetes"]},"status":{"phase":"Terminating"}} {"metadata":{"creationTimestamp":"2018-03-01T13:06:04Z","deletionTimestamp":"2018-03-01T13:08:03Z","name":"aks","resourceVersion":"44606555","selfLink":"/api/v1/namespaces/aks","uid":"4c32a517-1d51-11e8-900b-027f607c7b80"},"spec":{},"status":{"phase":"Terminating"}}
INFO: 2018/03/01 13:08:08.282695 EVENT DeleteNamespace {"metadata":{"creationTimestamp":"2018-03-01T13:06:04Z","deletionTimestamp":"2018-03-01T13:08:03Z","name":"aks","resourceVersion":"44606555","selfLink":"/api/v1/namespaces/aks","uid":"4c32a517-1d51-11e8-900b-027f607c7b80"},"spec":{},"status":{"phase":"Terminating"}}
DEBU: 2018/03/01 13:08:08.282777 removing default rule in namespace: aks, set weave-#[w{=fp4BYyel2u|{f;WvV5Wr
INFO: 2018/03/01 13:08:08.292115 destroying ipset: &npc.selectorSpec{key:"", selector:labels.internalSelector{}, dst:false, ipsetType:"hash:ip", ipsetName:"weave-usb0iYnjHs@=O7u8u/9gTbt%b", nsName:"aks"}
  • kubectl create ns aks - Same namespace created without any weave-npc crash. 🤘
INFO: 2018/03/01 13:11:59.809967 EVENT AddNamespace {"metadata":{"creationTimestamp":"2018-03-01T13:11:59Z","name":"aks","resourceVersion":"44607856","selfLink":"/api/v1/namespaces/aks","uid":"201bbdcd-1d52-11e8-ac8d-0238e90e49f8"},"spec":{"finalizers":["kubernetes"]},"status":{"phase":"Active"}}
INFO: 2018/03/01 13:11:59.813864 creating ipset: &npc.selectorSpec{key:"", selector:labels.internalSelector{}, dst:false, ipsetType:"hash:ip", ipsetName:"weave-usb0iYnjHs@=O7u8u/9gTbt%b", nsName:"aks"}
DEBU: 2018/03/01 13:11:59.817283 ensuring rule for DefaultAllow in namespace: aks, set weave-#[w{=fp4BYyel2u|{f;WvV5Wr

Need to make CI work but. CI is failing because the DeleteNetworkPolicy is deleting all the IPSets, and later the CI checks this: Should bring back the foo pod to default-allow as no netpol selects it

brb
brb reviewed Mar 2, 2018
View reviewed changes
npc/namespace.go Outdated
return err
}

// Flush defaultAllowIPSet

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

brb
brb reviewed Mar 3, 2018
View reviewed changes
npc/namespace.go Outdated
}

// Flush defaultAllowIPSet
err = ns.ips.Flush(ns.defaultAllowIPSet)

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

brb
brb reviewed Mar 3, 2018
View reviewed changes
npc/controller_test.go Outdated
return errors.Errorf("ipset %s does not exist", ipsetName)
} else if len(set.subSets) != 0 {

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@alok87
Copy link
Contributor Author

alok87 commented Mar 3, 2018

@brb all the above done, now the smoke tests are failing 🤔

rade
rade reviewed Mar 5, 2018
View reviewed changes
npc/namespace.go Outdated

if !ns.legacy {
// Delete defaultAllowIPSet
return ns.ips.Destroy(ns.defaultAllowIPSet)

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@rade
Copy link
Member

rade commented Mar 5, 2018 •
edited
Loading

the smoke tests are failing

That is expected. Their running is restricted so strangers cannot submit PRs containing bitcoin miners.

brb
brb reviewed Mar 5, 2018
View reviewed changes
npc/controller_test.go Outdated
@@ -79,7 +79,11 @@ func (i *mockIPSet) entryExists(ipsetName ipset.Name, entry string) bool {
}

func (i *mockIPSet) Flush(ipsetName ipset.Name) error {
return errors.New("Not Implemented")
if _, ok := i.sets[string(ipsetName)]; !ok {

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@brb
Copy link
Contributor

brb commented Mar 5, 2018

I've just pushed your branch to weaveworks/weave to trigger the CI build (https://circleci.com/gh/weaveworks/weave/10133).

@alok87 alok87 mentioned this pull request Mar 6, 2018
Closed
@alok87
Copy link
Contributor Author

alok87 commented Mar 7, 2018

@rade all done from my side. You would need to trigger CI again for smoke tests.

@rade
Copy link
Member

rade commented Mar 8, 2018

@brb ping

@brb brb added this to the 2.2.1 milestone Mar 8, 2018
@brb brb merged commit 42e4b9a into weaveworks:master Mar 8, 2018
@brb
Copy link
Contributor

brb commented Mar 8, 2018

Thanks for the contribution!

brb added a commit that referenced this pull request Mar 8, 2018
@brb
Merge pull request #3250 from alok87/del_default_ipset
b42895b 
Delete defaultAllowIPSet on namespace delete

Fix #3247

Duplicate merge commit as the PR got merged to "master" instead of "2.2"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rade rade rade left review comments

@brb brb brb approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@alok87 @rade @brb