Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Add -job to filename to match convention #321

Merged
merged 2 commits into from
Jan 2, 2020
Merged

Add -job to filename to match convention #321

merged 2 commits into from
Jan 2, 2020

Conversation

lkysow
Copy link
Member

@lkysow lkysow commented Jan 2, 2020

enterprise-licence.yaml => enterprise-license-job.yaml

@lkysow lkysow requested a review from a team January 2, 2020 14:54
Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

Looks good 👍 . Although we were rendering enterprise-license job template in one of the server tests, which I think is an error.

Is this a breaking change? This could break if you are using kustomize and helm template.

@@ -383,7 +383,7 @@ load _helpers
@test "server/StatefulSet: gossip encryption disabled in server StatefulSet when servers are disabled" {
cd `chart_dir`
local actual=$(helm template \
-x templates/enterprise-license.yaml \
-x templates/enterprise-license-job.yaml \
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 this should be templates/server-statefulset.yaml.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch. Actually that test doesn't make any sense because it's looking for certain keys within the server statefulset when servers are disabled. When servers are disabled then the statefulset isn't rendered at all so I just deleted the test.

This test was checking for config within the server statefulset when
servers were disabled. This didn't make sense because when servers are
disable the statefulset isn't rendered at all.
@lkysow lkysow merged commit 9cc2e9d into master Jan 2, 2020
@lkysow lkysow deleted the rename-job branch January 2, 2020 16:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants