Skip to content

Conversation

@RachitSharma2001
Copy link
Contributor

@RachitSharma2001 RachitSharma2001 commented May 1, 2023

Address #30851

I have added the following options to the kubernetes pod operator:

  • dns_config -> this allows the user to specify nameservers, searches, and options. This corresponds to the --dns, --dns-searches, and --dns-opt arguments for docker run
  • hostname -> corresponds to the --hostname argument for docker run
  • subdomain

@boring-cyborg boring-cyborg bot added provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:providers labels May 1, 2023
@RachitSharma2001
Copy link
Contributor Author

Sorry about that, the tests were failing before, but now they all pass.

security_context: dict | None = None,
container_security_context: dict | None = None,
dnspolicy: str | None = None,
dns_config: dict | None = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Kube has a type for this right?

Suggested change
dns_config: dict | None = None,
dns_config: V1PodDNSConfig | None = None,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I have pushed changes to reflect this.

)

@patch(HOOK_CLASS, new=MagicMock)
def test_pod_additional_options(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: something about this test name seems too generic, maybe:

Suggested change
def test_pod_additional_options(self):
def test_pod_dns_options(self):

Comment on lines +285 to +287
dns_config: k8s.V1PodDNSConfig | None = None,
hostname: str | None = None,
subdomain: str | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add these parameters to the docstring above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I had forgotten to add them. I have added them now.

@RachitSharma2001
Copy link
Contributor Author

Hi everyone, I was wondering if there was anything else I should add to this PR or if these changes look good?

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

Labels

area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants