-
Notifications
You must be signed in to change notification settings - Fork 13
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
ROX-20450: Configure operator resources in chart #1417
ROX-20450: Configure operator resources in chart #1417
Conversation
defaultValues := resourcesChart.Values["operator"].(map[string]interface{}) | ||
defaultResources := defaultValues["resources"].(map[string]interface{}) | ||
defaultRbacResources := defaultValues["rbac"].(map[string]interface{}) | ||
|
||
chartVals := chartutil.Values{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an obvious room for improvement in how to chart values constructed but I would prefer a separate PR for this
@@ -49,11 +49,11 @@ spec: | |||
protocol: TCP | |||
resources: | |||
limits: | |||
cpu: 2 | |||
memory: 1Gi | |||
cpu: {{ ((((.rbac).resources).limits).cpu) | default $.Values.operator.rbac.resources.limits.cpu }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parenthesis is an additional nil handlers in case not correct resource structure is passed to the helm chart render from gitops config
/retest e2e |
@SimonBaeumer: The
Use In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/test e2e |
…figure_operator_resources_in_chart
@@ -45,7 +45,7 @@ func validateCRDUrls(path *field.Path, urls []string) field.ErrorList { | |||
|
|||
func validateCRDURL(path *field.Path, urlStr string) field.ErrorList { | |||
var errs field.ErrorList | |||
if _, err := url.Parse(urlStr); err != nil { | |||
if _, err := url.ParseRequestURI(urlStr); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parse
doesn't complain about broken url
string
@@ -54,13 +54,18 @@ func RenderChart(operators OperatorConfigs) ([]*unstructured.Unstructured, error | |||
for _, operator := range operators.Configs { | |||
valuesArr = append(valuesArr, chartutil.Values(operator)) | |||
} | |||
|
|||
resourcesChart, err := charts.GetChart("rhacs-operator", operators.CRDURLs) | |||
operatorValues := resourcesChart.Values["operator"].(map[string]interface{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to safe-cast here,
e.g
operatorValues, ok := resourcesChart.Values["operator"].(map[string]interface{})
if !ok { return fmt.Errorf("...") }
defaultValues, ok := ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor adjustment for safe casting
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kurlov, ludydoo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Allow configure operator resource via chart values.
Overrides can be passed via gitops config. If not presented the default values from
values.yaml
will be usedAlso fail fast in case broken CRD URL provided
Checklist (Definition of Done)
- [ ] Unit and integration tests addedTest manual
- [ ] Documentation added if necessary (i.e. changes to dev setup, test execution, ...)ROX-12345: ...
- [ ] Discussed security and business related topics privately. Will move any security and business related topics that arise to private communication channel.- [ ] Add secret to app-interface Vault or Secrets Manager if necessary- [ ] RDS changes were e2e tested manually- [ ] Check AWS limits are reasonable for changes provisioning new resourcesTest manual