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

error out when configurations do not match #418

Merged
merged 2 commits into from
Oct 17, 2024

Conversation

nabuskey
Copy link
Collaborator

@nabuskey nabuskey commented Oct 17, 2024

fixes: #416

if a cluster was created with a configuration, then a different configuration is passed for the same cluster, error out.

./idpbuilder create
./idpbuilder create --use-path-routing
Oct 17 20:39:41 INFO Creating kind cluster logger=setup
Oct 17 20:39:41 INFO Runtime detected logger=setup provider=docker
Oct 17 20:39:41 INFO Cluster already exists logger=setup cluster=localdev
Oct 17 20:39:41 INFO Adding CRDs to the cluster logger=setup
Oct 17 20:39:41 INFO Setting up CoreDNS logger=setup
Oct 17 20:39:41 INFO Setting up TLS certificate logger=setup
Oct 17 20:39:41 ERROR Error while checking incompatible flags logger=setup err=provided command flags and existing configurations are incompatible. please recreate the cluster. existing: {Protocol:https Host:cnoe.localtest.me IngressHost:cnoe.localtest.me Port:8443 UsePathRouting:false SelfSignedCert:}, given: {Protocol:https Host:cnoe.localtest.me IngressHost:cnoe.localtest.me Port:8443 UsePathRouting:true SelfSignedCert:}
Error: :provided command flags and existing configurations are incompatible. please recreate the cluster. existing: {Protocol:https Host:cnoe.localtest.me IngressHost:cnoe.localtest.me Port:8443 UsePathRouting:false SelfSignedCert:}, given: {Protocol:https Host:cnoe.localtest.me IngressHost:cnoe.localtest.me Port:8443 UsePathRouting:true SelfSignedCert:}

Signed-off-by: Manabu McCloskey <[email protected]>
@nabuskey nabuskey requested a review from blakeromano October 17, 2024 20:42
Copy link
Contributor

@blakeromano blakeromano left a comment

Choose a reason for hiding this comment

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

overall looks good just some questions about types we are choosing

Host string `json:"host,omitempty"`
IngressHost string `json:"ingressHost,omitempty"`
Port string `json:"port,omitempty"`
UsePathRouting bool `json:"usePathRouting,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I mean maybe a nit but do we ever expect to have other kind of routing where we want to make this an enum string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm I honestly cannot think of other ways to route for now. Maybe this is something we can take a look at later along with the potential changes to the name of the flag. #417

api/v1alpha1/localbuild_types.go Show resolved Hide resolved
@nabuskey nabuskey merged commit edaa249 into cnoe-io:main Oct 17, 2024
5 checks passed
@nabuskey nabuskey deleted the fail-early branch October 17, 2024 21:08
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.

Refuse to run when incomatible settings are used
2 participants