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

permit specifying daemonset and deployment resources #930

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

clementnuss
Copy link

Change description

permit specifying trident-controller deployment and trident-node-linux daemonset container resource requests/limits in the TridentOrchestrator spec.

Did you add unit tests? Why not?

Yes, for both modifications of the deployment and daemonset

Does this code need functional testing?

No, I have already tested the functionality locally, and the following works:

  • not specifying e.g. daemonsetResources (or specyifying a {}), in which case all containers are configures with a resources: {} spec.
  • specifying a proper resources dict, i.e. something such as
    requests:
      cpu: 14mi
    in which case the containers do get the correct request
  • specifying an incorrect value in the TridentOrchestrator spec, in which case trident-orchestrator correctly reports that the configuration is not readable.

Is a code review walkthrough needed? why or why not?

Probably needed, I'm typically not sure whether my modifications to types.go file needs some autogen of some sort.

Should additional test coverage be executed in addition to pre-merge?

don't think so

Does this code need a note in the changelog?

permit specyfing daemonset and deployment resource requests and limits.

Does this code require documentation changes?

don't think so

Additional Information

I was sure whether I should offer the ability to specify the resources for each container of the deployment/daemonset. that would probably amount to 6-7 resources spec, let me know if you want me to implement that.

closes #853

@clementnuss
Copy link
Author

just rebased on master. could you consider this ? it's hard to keep PRs open on the long run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No limits or requests set on Trident resources
1 participant