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

fix code and note blocks in dualtor pages #1649

Merged
merged 3 commits into from
Sep 17, 2024

Conversation

aaaaaaaalex
Copy link
Contributor

@aaaaaaaalex aaaaaaaalex commented Sep 11, 2024

Product Version(s):
mdx syntax bugfix applies to all versions.
New documentation applies only to 3.20EP2 and later.

Issue:
PMREQ-629

Link to docs preview:

SME review:

  • An SME has approved this change.

DOCS review:

  • A member of the docs team has approved this change.

Additional information:

Merge checklist:

  • Deploy preview inspected wherever changes were made
  • Build completed successfully
  • Test have passed

@aaaaaaaalex aaaaaaaalex requested a review from a team as a code owner September 11, 2024 10:50
Copy link

netlify bot commented Sep 11, 2024

Deploy Preview succeeded!

Built without sensitive environment variables

Name Link
🔨 Latest commit 353927b
🔍 Latest deploy log https://app.netlify.com/sites/tigera/deploys/66e7fb735d4ab100085983d4
😎 Deploy Preview https://deploy-preview-1649--tigera.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 25 (🔴 down 23 from production)
Accessibility: 90 (no change from production)
Best Practices: 75 (no change from production)
SEO: 93 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Sep 11, 2024

Deploy Preview for calico-docs-preview-next ready!

Name Link
🔨 Latest commit 353927b
🔍 Latest deploy log https://app.netlify.com/sites/calico-docs-preview-next/deploys/66e7fb732e344f0008bb22b6
😎 Deploy Preview https://deploy-preview-1649--calico-docs-preview-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 40 (🔴 down 16 from production)
Accessibility: 90 (no change from production)
Best Practices: 83 (no change from production)
SEO: 86 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@ctauchen ctauchen 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 this! A few comments for you.

@@ -475,6 +473,7 @@ startup prior to receiving BGPPeer and BGPConfiguration resources from the datas
EarlyNetworkingConfiguration will be superseded by any BGPPeer or BGPConfiguration
resources after successful startup.

For a full description of the EarlyNetworkingConfiguration resource, see [here](../../reference/resources/earlynetworkconfiguration.mdx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
For a full description of the EarlyNetworkingConfiguration resource, see [here](../../reference/resources/earlynetworkconfiguration.mdx)
For a full description, see [Early Network Configuration](../../reference/resources/earlynetworkconfiguration.mdx)

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the difference between EarlyNetworkingConfiguration (line 473) and EarlyNetworkConfiguration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EarlyNetworkingConfiguration doesn't exist - typo!

Early network configuration will be superseded by any BGPPeer or BGPConfiguration
resources after successful startup.

For a detailed guide on how to set up dual ToR, see [here](../../networking/configuring/dual-tor.mdx).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
For a detailed guide on how to set up dual ToR, see [here](../../networking/configuring/dual-tor.mdx).
For a detailed guide on how to set up dual ToR, see [Deploy a dual ToR cluster](../../networking/configuring/dual-tor.mdx).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Always good practice to have specific anchors.

Comment on lines 15 to 18
:::note
Early network configurations are intended for use only when {{prodname}} is being run in "early networking" mode, i.e. when operating a "dual ToR" networking configuration. If you are not running a dual-plane cluster, this is probably not what you are looking for!
For configuring BGP networking under normal circumstances (when {{prodname}} is running), see [BGPConfiguration](bgpconfig.mdx), [BGPPeer](bgppeer.mdx), and [BGPFilter](bgpfilter.mdx).
:::
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider moving this information into the top paragraph. If it's really only for dual-tor, you can say that in the first description.

### ConfigStableAddress
| Field | Description | Accepted Values | Schema Default |
| ------------------------------------- | --------------------------------------------------------------- | ----------------- | -------------- |
| address | IP address which {{prodname}} should provision during earlynetworking as the "stable" address | IP Address string | "" |
Copy link
Collaborator

Choose a reason for hiding this comment

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

during earlynetworking --> during early networking

If that's the established wording. Sounds a bit funny to my ear.

| Field | Description | Accepted Values | Schema Default |
| ------------ | --------------------------------------------------------------------------------------------------------- | ----------------- | -------------- |
| peerIP | The IP address of the ToR | IP Address string | "" |
| peerASNumber | The AS number of the ToR. When left as 0, {{prodname}} early-networking will use its own node's AS number | integer | 0 |
Copy link
Collaborator

Choose a reason for hiding this comment

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

early networking

@aaaaaaaalex
Copy link
Contributor Author

Feedback is all implemented now, reviewdog is finding English typos within YAML, safe to ignore.

@ctauchen
Copy link
Collaborator

LGTM, thanks!

@ctauchen ctauchen merged commit 00792be into tigera:main Sep 17, 2024
9 of 10 checks passed
@ctauchen ctauchen mentioned this pull request Sep 19, 2024
5 tasks
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.

2 participants