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

drenv: Fix submariner for MacOS #1497

Closed

Conversation

raghavendra-talur
Copy link
Member

On Mac, the check for certs is more strict and it fails for submariner
service. Turning off the check for certs.

More info: golang/go#51991

Signed-off-by: Raghavendra Talur raghavendra.talur@gmail.com

On Mac, the check for certs is more strict and it fails for submariner
service. Turning off the check for certs.

More info: golang/go#51991

Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Copy link
Contributor

@asn1809 asn1809 left a comment

Choose a reason for hiding this comment

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

lgtm

"""
Run subctl join ... logging progress messages.
"""
args = ["join", broker_info, "--context", context, "--clusterid", clusterid]
args.append(f"--check-broker-certificate={check_broker_certificate}")
Copy link
Member

Choose a reason for hiding this comment

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

This will add:

--check-broker-certificate=True

Which may not work since it should use "true|false".

Since by default subctl checks certs, we can do this:

if not check_broker_certificates:
    args.append("--check-broker-certificate=false")

Copy link
Member

Choose a reason for hiding this comment

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

I included this change in #1536

f7bb75d

@@ -60,6 +60,7 @@ def join_cluster(cluster, broker_info):
clusterid=cluster,
cable_driver="vxlan",
version=VERSION,
check_broker_certificate=False,
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to do this only on macOS? It would be nice to detect real issues with certificates when we can.

@raghavendra-talur
Copy link
Member Author

Closing as it was fixed in #1536

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.

3 participants