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

feat(ctl): Namespace separation and support skipping release installation #79

Merged
merged 19 commits into from
Aug 9, 2023

Conversation

Techassi
Copy link
Member

@Techassi Techassi commented Aug 3, 2023

@Techassi Techassi self-assigned this Aug 3, 2023
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

Not tested it yet, waiting for non-draft PR with that

rust/stackable-cockpit/src/constants.rs Outdated Show resolved Hide resolved
rust/stackable-cockpit/src/constants.rs Show resolved Hide resolved
rust/stackablectl/src/args/namespace.rs Outdated Show resolved Hide resolved
rust/stackablectl/src/args/namespace.rs Outdated Show resolved Hide resolved
rust/stackablectl/src/args/repo.rs Outdated Show resolved Hide resolved
rust/stackablectl/src/cli/mod.rs Outdated Show resolved Hide resolved
rust/stackablectl/src/args/namespace.rs Outdated Show resolved Hide resolved
rust/stackablectl/src/cmds/operator.rs Outdated Show resolved Hide resolved
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

Haven't had the time to actually use and test it yet :/
The final missing pice from stackabletech/stackablectl#259 would be

Demos have the possibility to say "i can only run in stackable-demo" namespace (in stacks.yaml or demo.yaml). If they do so, a warning is printed when they are installed in a different namespace, saying the demo will likely not work. This can be the case as we have to use FQDN service names so that the TLS certs are valid

We could do it in a follow-up PR but I think it should not be that complicated

rust/stackable-cockpit/src/kube.rs Outdated Show resolved Hide resolved
rust/stackable-cockpit/src/kube.rs Outdated Show resolved Hide resolved
rust/stackable-cockpit/src/platform/namespace.rs Outdated Show resolved Hide resolved
rust/stackable-cockpit/src/platform/stack/spec.rs Outdated Show resolved Hide resolved
rust/stackablectl/README.md Show resolved Hide resolved
Techassi and others added 3 commits August 7, 2023 12:04
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

cargo r -p stackablectl -- stack in trino-superset-s3 -c kind
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
     Running `target/debug/stackablectl stack in trino-superset-s3 -c kind`
Error: stack command error

Caused by these errors (recent errors listed first):
  1: stack error
  2: release install error *
  3: failed with Helm error *
  4: failed to install Helm release *
  5: helm error: create: failed to create: namespaces "stackable-operators" not found

NOTE: Some redundant information has been removed from the lines marked with *. Set SNAFU_RAW_ERROR_MESSAGES=1 to disable this behavior.

@Techassi Techassi requested a review from sbernauer August 7, 2023 13:09
@sbernauer
Copy link
Member

Change LGTM, did play around and bit and gave feedback via PN

Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

Whup whup

Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

The only thing missing from stackabletech/stackablectl#259 is:

stackablectl tries to create specified namespaces if not existing

  • If this fails because of missing permissions an error is thrown as the user is suggested to specify operators and demo namespace manually

@Techassi Techassi requested a review from sbernauer August 8, 2023 15:07
sbernauer
sbernauer previously approved these changes Aug 9, 2023
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

@Techassi Techassi enabled auto-merge August 9, 2023 07:37
@Techassi Techassi added this pull request to the merge queue Aug 9, 2023
Merged via the queue into main with commit 581ebad Aug 9, 2023
25 checks passed
@Techassi Techassi deleted the feature/namespaces branch August 9, 2023 07:51
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.

Change default namespaces + Support skipping release installation
3 participants