fix(config/orca): set global.spinnaker.timezone for orca #1117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
maggieneterval
commented
Dec 4, 2018
maggieneterval
commented
- Fixes bug where Restrict Execution During Time Window stages always calculate whether the current time is within range using PST rather than a user's configured timezone
jtk54
approved these changes
Dec 4, 2018
.../main/java/com/netflix/spinnaker/halyard/deploy/spinnaker/v1/profile/OrcaProfileFactory.java
Outdated
Show resolved
Hide resolved
maggieneterval
force-pushed
the
orca-timezone-fix
branch
from
b651a16
to
b4d8d33
Compare
dbyron0
pushed a commit
to dbyron0/halyard
that referenced
this pull request
Jan 8, 2019
maggieneterval
added a commit
to maggieneterval/kleat
that referenced
this pull request
Jun 2, 2020
At least Deck, Orca, and Echo consume a user-configurable timezone. Unfortunately, Halyard currently writes the timezone to a `global.spinnaker.timezone` field in spinnaker.yml, which is [my fault](spinnaker/halyard#1117), though in my defense it was only after a CR suggestion that I moved the translation from Orca's profile to the global profile... I don't want to further complicate this Deck-focused PR with logic to add timezone to Echo and Orca's protos, so will add a TODO to do that in a followup PR.
mergify bot
added a commit
to spinnaker/kleat
that referenced
this pull request
Jun 2, 2020
* feat(proto): add Bearychat, Email, Google Chat, and Pubsub notifications protos Although these four notification types are not currently configurable with Halyard, they are defaulted to enabled in Deck's halconfig/settings.js. This is because when *any* notification types are configured, Deck assumes that is the exhaustive list of enabled notification types. (If *no* notification types are configured, Deck's default behavior is to enable all of them.) Since it is unclear if it would be reasonable to push the existing Halyard-default-behavior into Deck, let's simply be consistent with notification types in Kleat and allow users to manually enable any types they wish. * feat(proto): add canary UI protos Adds Deck-specific protos to canary.proto. Note that the defaults for `stagesEnabled` and `defaultJudge` depend on those being pushed from Halyard to Deck-Kayenta in [this PR](spinnaker/deck-kayenta#499). * feat(proto): add timezone proto to halconfig At least Deck, Orca, and Echo consume a user-configurable timezone. Unfortunately, Halyard currently writes the timezone to a `global.spinnaker.timezone` field in spinnaker.yml, which is [my fault](spinnaker/halyard#1117), though in my defense it was only after a CR suggestion that I moved the translation from Orca's profile to the global profile... I don't want to further complicate this Deck-focused PR with logic to add timezone to Echo and Orca's protos, so will add a TODO to do that in a followup PR. * feat(proto): update features.proto Updates features.proto to include flags consumed by Deck. Adds TODO for how to best handle roscoMode, which Deck's halconfig/settings.js currently sets to true (Halyard does not expose a command to manually set roscoMode.). * feat(proto): translate Gremlin integration to Gate Also adds tests. * feat(proto): add Deck protos Strongly types all Deck configuration exposed by Halyard, with a few exceptions: * canary.atlasWebComponentsUrl: likely only used by Netflix, unclear why this is Halyard-configurable. * canary.reduxLogger: should only be useful in local development, unclear why this is Halyard-configurable. * defaultInstancePort: defaulted in halconfig/settings to 80. In most places in the code, we also fall back to 80, but there are a few places we fallback to null instead, perhaps to force users to think about making a choice here, for example, when configuring a new application. Let's not expose this property in Kleat until presented with a compelling reason. Also adds several TODOs to handle after some discussion in a followup PR: * Handle top-level version * Handle changelog gist * Handle roscoMode * Add Tencent cloud protos * feat(pkg): add hal to deck translation Adds TODOS for the following: * Replace stubbed gateUrl with actual gateUrl, which may be http or http depending on other settings * Add changelog translation once we determine how we are going to handle * Refactor getDeckProvidersConfig which feels like terrible code, but couldn't find an elegant Go-native way to reduce duplication Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.