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

Create tigera-dpi namespace irrespective of DPI resource #1516

Merged
merged 1 commit into from
Sep 14, 2021

Conversation

Suraiya-Hameed
Copy link
Contributor

Description

Issue: Applying network policies fails during CaliEnt install as tigera-dpi namespace doesn't exist unit DeepPacketInspection resource is created.

This PR is to create a tigera-dpi namespace without depending on DeepPacketInspection resource.

For PR author

  • Tests for change.
  • If changing pkg/apis/, run make gen-files
  • If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

I don't think this PR fixes what it is suppose to fix. I think to ensure that policies will be able to be created I think namespace always needs to be created.
I think there should be a test that with only IntrusiontDetection CR that the dpi namespace should be created. Right now as I read it, if there is no license or dpi resource then the namespace will not be created (and will be deleted in the no license case).
I think the render should always return the namespace as being created so the policy will always be able to be applied, even in the no license case.

@Suraiya-Hameed
Copy link
Contributor Author

@tmjd I've update the PR so dpi namespace is created even if there is no dpi resource. As discussed I haven't updated the condition where dpi namespace is deleted if there is no license as all other components are doing the same.

r.status.SetDegraded(fmt.Sprintf("Waiting for %s secrets to be available", render.NodeTLSSecretName), "")
return reconcile.Result{}, err
}
nodeTLSSecret, err = utils.GetSecret(ctx, r.client, render.NodeTLSSecretName, rmeta.OperatorNamespace())
Copy link
Member

Choose a reason for hiding this comment

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

It shouldn't be necessary to get this and the following secrets and configmap if there is no DPI.
I see why you did that but it isn't necessary in the delete case for the render to have the full secrets and configmap (with data), when deleting it only needs the names and namespaces of the object to delete. WDYT about putting the controller code back to the way you had it and adjusting the render to create the empty secret and configmaps when it needs to delete the resources?

Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

LGTM

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.

None yet

4 participants