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

When creating a pod, switching between config view and yaml view breaks the pod spec #10171

Open
mantis-toboggan-md opened this issue Dec 15, 2023 · 2 comments · May be fixed by #10208
Open
Assignees
Labels
area/workloads kind/bug kind/design kind/tech-debt Technical debt QA/manual-test Indicates issue requires manually testing
Milestone

Comments

@mantis-toboggan-md
Copy link
Member

mantis-toboggan-md commented Dec 15, 2023

To reproduce:

  1. Navigate to the pod creation page (/dashboard/c//explorer/pod/create)
  2. Add a name, container name, and container image
  3. Click 'Edit as YAML'

Result:
The pod now has spec.template.spec.containers defined instead of spec.containers and creation fails.

Additional Context
spec.template.spec.containers would be correct for a workload. There appear to be some shenanigans going on behind-the-scenes with pod creation/editing in order to use the same form as workloads.
https://github.com/rancher/dashboard/blob/master/shell/edit/workload/mixins/workload.js#L181-L202

@rak-phillip
Copy link
Member

rak-phillip commented Dec 21, 2023

There appears to be quite a few issues with the "Edit as YAML" button for the Pod form. Most of these problems have to deal with design intricacies of incorporating a Workload (Pod) into a form primarily designed for Workload Resources (Deployments, ReplicaSet, StatefulSets, etc.) 1.

Clicking "Edit as YAML" generates an invalid pod spec while both creating and editing a pod. This issue seems to trace back to the form's inception 2. As mentioned in the issue description, the root cause lies in the form's expectation of data shaped as spec.template.spec instead of spec, but I don't think the change to resolve this will be as simple as modifying the workload mixin.

To address this problem in the form, we currently mutate the shape of spec just before saving, allowing the pod to save properly 3.

I'm considering several approaches to tackle this issue:

  1. Mutate value in shell/edit/workload/index.vue

    • This can probably be achieved in the willSaveWorkload hook located under shell/edit/workload/mixins/workload.js
    • Doing this will have some knock-on effects, as mutating value will break existing form fields
  2. Expose a Slot in CruResource for the YAML Editor

    • This can allow us to pass a sanitized copy of value directly to the yaml editor
    • It might expose some implementation details that aren't relevant to Pods

I'm not sold on either approach at the moment and I think this problem requires more thought and design before moving forward to fix it. I'll continue to investigate and will follow up with more details as I learn more.

Footnotes

  1. https://kubernetes.io/docs/concepts/workloads/

  2. https://github.com/rancher/dashboard/commit/35c51c7f47192251a47b5010ce25d43c0747bb86#diff-ac776780d70ceae4eafb989d0dcd9a13815410a801c040143e9f60492d7aeb4cR160-R162

  3. https://github.com/rancher/dashboard/blob/master/shell/models/pod.js#L236-L248

@rak-phillip rak-phillip linked a pull request Jan 2, 2024 that will close this issue
@nwmac nwmac added the QA/manual-test Indicates issue requires manually testing label Jan 30, 2024
@gaktive gaktive modified the milestones: v2.9.0, v2.10.0 Jun 28, 2024
@nwmac nwmac modified the milestones: v2.10.0, v2.11.0 Jul 4, 2024
@nwmac nwmac modified the milestones: v2.12.0, v2.11.0 Nov 1, 2024
@gaktive gaktive added the kind/tech-debt Technical debt label Dec 4, 2024
@gaktive
Copy link
Member

gaktive commented Dec 4, 2024

This will be a big thing to tackle since we should refactor how we handle pods.

@gaktive gaktive modified the milestones: v2.11.0, v2.12.0 Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/workloads kind/bug kind/design kind/tech-debt Technical debt QA/manual-test Indicates issue requires manually testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants