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

Bugfix: host_alias to host_aliases #413

Closed
wants to merge 1 commit into from
Closed

Conversation

minac
Copy link
Contributor

@minac minac commented Apr 23, 2019

Bugfix: host_alias to host_aliases

Bugfix: host_alias to host_aliases
@pdecat
Copy link
Contributor

pdecat commented Apr 25, 2019

In order to lighten the documentation maintenance burden, I believe we should remove the duplicated pod spec documentation from the kubernetes_daemon_set and kubernetes_deployment resources and just refer to the kubernetes_pod documentation, similar to what's done for kubernetes_replication_controller and kubernetes_stateful_set.

@alexsomesan WDYT?

Notes:

  • in the case of kubernetes_daemon_set and kubernetes_stateful_set resources, the whole template block comes from the pod schema using the podTemplateFields() function which relies on the namespacedMetadataSchema() and podSpecFields() functions.
  • in the case of kubernetes_deployment, only spec.template.spec comes directly from the pod schema using the podSpecFields() function, the spec.template.metadata comes from metadataSchema() (not namespacedMetadataSchema()).
  • in the case of kubernetes_replication_controller, the whole template block comes from the pod schema using the replicationControllerTemplateFieldSpec() function which relies on the namespacedMetadataSchema() and podSpecFields() functions and implements support for legacy fields which were misplaced (see Add pod metadata to replication controller spec template #193).

Some refactoring there could help improve the situation further.

@minac
Copy link
Contributor Author

minac commented Apr 25, 2019

Although I completely agree with the comments, aren't we making the scope of this simple PR way too big? Maybe a separate issue would be better to make improvements/fixes in other locations?
I know I wasted hours of my day because of this one bug and can imagine others are suffering from the same problem.

@pdecat
Copy link
Contributor

pdecat commented Apr 25, 2019

Agreed. Maybe just address #413 (comment) here.
We'll take care of the rest in another issue/PR.

@pdecat
Copy link
Contributor

pdecat commented Apr 25, 2019

@minac
Copy link
Contributor Author

minac commented Apr 25, 2019

Hey @pdecat, we're all humans here! Let's just get this fixed using the magic of open source docs :)

@alexsomesan
Copy link
Member

In order to lighten the documentation maintenance burden, I believe we should remove the duplicated pod spec documentation from the kubernetes_daemon_set and kubernetes_deployment resources and just refer to the kubernetes_pod documentation, similar to what's done for kubernetes_replication_controller and kubernetes_stateful_set.

@alexsomesan WDYT?

Notes:

  • in the case of kubernetes_daemon_set and kubernetes_stateful_set resources, the whole template block comes from the pod schema using the podTemplateFields() function which relies on the namespacedMetadataSchema() and podSpecFields() functions.
  • in the case of kubernetes_deployment, only spec.template.spec comes directly from the pod schema using the podSpecFields() function, the spec.template.metadata comes from metadataSchema() (not namespacedMetadataSchema()).
  • in the case of kubernetes_replication_controller, the whole template block comes from the pod schema using the replicationControllerTemplateFieldSpec() function which relies on the namespacedMetadataSchema() and podSpecFields() functions and implements support for legacy fields which were misplaced (see Add pod metadata to replication controller spec template #193).

Some refactoring there could help improve the situation further.

Patrick, I agree. We should unify the pod templates across all workload resources and make everything re-usable. I would love that.
However the provider is still missing features and entire resources, so priority has to go to that on my side until we have decent coverage.

Happy to look at PRs though 🤓

aareet added a commit to aareet/terraform-provider-kubernetes that referenced this pull request Jun 2, 2020
@aareet
Copy link
Contributor

aareet commented Jun 2, 2020

Closing in favour of #869 which fixes the issue in the daemonset & pod docs as well. I retained the original commit and author from this PR.

@aareet aareet closed this Jun 2, 2020
aareet added a commit that referenced this pull request Jun 15, 2020
* Bugfix: host_alias to host_aliases

Bugfix: host_alias to host_aliases

* Update pod and daemonset documentation to reflect host_aliases

Per #413 (comment)

Co-authored-by: Miguel David <[email protected]>
@ghost
Copy link

ghost commented Jul 3, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Jul 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants