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

fix(halyard): Bugfix 6112: Populate settings.js with default settings for Oracle provider #1813

Closed
wants to merge 1 commit into from

Conversation

anupmpatil
Copy link

fix(halyard): settings.js doesn't have Oracle default settings
This settings.js is being used as template by halyard and gets hydrated with default settings. Without these settings, oracle default settings were not getting populated, causing at least one know failure while creating OCI load balancer template.

After this fix, default settings for oracle provider will be populated properly

@anupmpatil anupmpatil changed the title fix(halyard): Populate settings.js with default settings for Oracle provider fix(halyard): Bugfix 6112: Populate settings.js with default settings for Oracle provider Oct 16, 2020
@anupmpatil
Copy link
Author

anupmpatil commented Oct 16, 2020

@maggieneterval Could you please review this PR? I read in one of the comments in other PR (#1598) that the plan is for sunsetting Halyard. What's the timeline for that to happen and other tool to come in? Just asking as information. Does that mean that in future no more fixes will be needed in Halyard?

@maggieneterval
Copy link
Contributor

Hi @anupmpatil! We've been trying to move config defaults into the microservices themselves rather than have Halyard supply them. Can you please set these defaults in Deck's Oracle provider instead?

Feel free to follow the Kleat repo and join the #kleat Spinnaker Slack channel for updates on Halyard's replacement (no definite timeline yet).

Thanks!

@anupmpatil
Copy link
Author

@maggieneterval Thank you for the explanation, I have made changes in Deck's Oracle provider, I will ensure if any more changes are needed in Deck. So, just to get more clarification regarding this fix, do we still need to go ahead and merge it?

@maggieneterval
Copy link
Contributor

@anupmpatil Thanks for letting me know; if you've already handled these defaults in Deck's Oracle provider then I'll go ahead and close this PR.

@anupmpatil
Copy link
Author

@maggieneterval
Till the alternative service comes up, the users who will (or are still using) use halyard with oracle provider are still going to see the issues because they will not have default settings and will see the same bug. I have 2 questions regarding this:

  1. Can we request to let this change in and in case if it does goes in, are you planning any new halyard version release that accommodate this?
  2. In case this change does not go in, we have work around using the settings-local.js configuration. We can create document explaining the steps needed for this work around. Where should we publish this document, so that users trying to use oracle provider with halyard can clearly know how they should configure settings-local.js?

@maggieneterval
Copy link
Contributor

Till the alternative service comes up, the users who will (or are still using) use halyard with oracle provider are still going to see the issues because they will not have default settings

Sorry if my previous comment wasn't totally clear; I meant that we should handle setting these Oracle default settings in Deck itself (wherever we are reading them in the Deck code), rather than having Halyard write them or having users configure them in their settings-local.js.

Kleat will also not set any defaults for users; see here for more details.

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.

2 participants