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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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 [email protected]

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 <[email protected]>
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.

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