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

Make Instance name deterministic #960

Merged
merged 1 commit into from
Oct 24, 2019

Conversation

yankcrime
Copy link
Member

@yankcrime yankcrime commented Oct 17, 2019

What this PR does / why we need it:
Remove the random string suffixed onto the end of instance names, instead using a suffix of
-instance. Users can use the --instance option to provide a unique name if they so wish.

Fixes #948

@alenkacz
Copy link
Contributor

@yankcrime nice! You also need to change the description I adjusted with #943

@yankcrime
Copy link
Member Author

Ah good shout, thanks @alenkacz!

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

LGTM. I added label so we don't forget to mention it in changelog

Copy link
Member

@nfnt nfnt left a comment

Choose a reason for hiding this comment

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

This looks great, my only concern is parallel test execution. For that, unique instance names are a must.

pkg/kudoctl/packages/package.go Outdated Show resolved Hide resolved
pkg/kudoctl/packages/package_test.go Outdated Show resolved Hide resolved
@yankcrime yankcrime force-pushed the deterministic_instance_names branch 2 times, most recently from ba7ea39 to 91aa068 Compare October 17, 2019 13:09
@yankcrime yankcrime requested a review from nfnt October 17, 2019 13:15
pkg/kudoctl/packages/package.go Outdated Show resolved Hide resolved
@yankcrime yankcrime force-pushed the deterministic_instance_names branch 2 times, most recently from 4b78cfa to 3458af5 Compare October 22, 2019 10:30
Copy link
Contributor

@zen-dog zen-dog left a comment

Choose a reason for hiding this comment

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

🚢

Remove the random string suffixed onto the end of instance names,
instead using a suffix of `-instance`.  Users can use the `--instance`
option to provide a unique name if they so wish.

The removal of the random string suffixed onto instance names means
that the associated test can be simplified.
@alenkacz alenkacz merged commit 3ce18a5 into kudobuilder:master Oct 24, 2019
@yankcrime yankcrime deleted the deterministic_instance_names branch October 24, 2019 10:16
yankcrime added a commit to yankcrime/operators that referenced this pull request Nov 4, 2019
With the introduction of [deterministic instance
names](kudobuilder/kudo#960) we can simplify
some of the starting defaults such as the connection string for
ZooKeeper.
zmalik pushed a commit to kudobuilder/operators that referenced this pull request Nov 5, 2019
With the introduction of [deterministic instance
names](kudobuilder/kudo#960) we can simplify
some of the starting defaults such as the connection string for
ZooKeeper.
@porridge porridge added release/highlight This PR is a highlight for the next release and removed release-highlight labels Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/highlight This PR is a highlight for the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deterministic instance names for operators
6 participants