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

add hostAlias section to data loader #59

Merged
merged 6 commits into from
Mar 9, 2022
Merged

Conversation

gchickma
Copy link
Member

@gchickma gchickma commented Mar 1, 2022

Add hostAlias section to data loader

@tlawrie
Copy link
Member

tlawrie commented Mar 1, 2022

This is probably a Flow 3.8.0 change as it's not been verified in all the Flow 3.7.0 changes so far.

@gchickma
Copy link
Member Author

gchickma commented Mar 2, 2022

Change is very minor, i.e. it includes an optional hostAlias block to the loader pod descriptor.
Have tested it in other charts similar to Flow and works as expected.
If it can be merged in I am happy to test as part of 3.7.0.

@tlawrie
Copy link
Member

tlawrie commented Mar 2, 2022

My preference is that it waits until 3.8.0 as 3.7.0 has been in draft release for 2 weeks awaiting on one front end tag. However, we now have multiple chart and worker tags that are trying to be added with creep in the release scope.

However, if it was agreed in the community meeting, then we can add but we would need the documentation updated ASAP and README updated.

@tlawrie
Copy link
Member

tlawrie commented Mar 2, 2022

Also has it been tested

  • new install without host alias
  • new install with host alias
  • upgrade to an install without host alias
  • upgrade to an install with host alias

The TLS change required two last-minute emergency fixes in release shakeout so im trying to make sure that we don't ship with a faulty install.

@gchickma
Copy link
Member Author

gchickma commented Mar 2, 2022

There's no documentation currently for hostAliases in the README.md
I will add a new section

@gchickma
Copy link
Member Author

gchickma commented Mar 2, 2022

Wrt testing, yes to all for similar chart, no to Flow as it hasn't been merged and packaged as yet

@gchickma
Copy link
Member Author

gchickma commented Mar 2, 2022

Updated chart pushed

@gchickma
Copy link
Member Author

gchickma commented Mar 8, 2022

Thx @morarucostel @tlawrie for the reviews. Please let me know when we can merge this PR. Thx.

@timrbula
Copy link
Member

timrbula commented Mar 9, 2022

@gchickma With the two approvals, lets merge this in?

@gchickma gchickma merged commit 6bef577 into main Mar 9, 2022
@timrbula timrbula deleted the addHostAliasToHookJob branch March 9, 2022 03:25
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.

4 participants