Misc. minor corrections, including: #39

Fredi-raspall · 2024-09-30T07:56:09Z

Unified some formatting.
removed spurious ```yaml that was being rendered.
Fixed the definitions for VLANNamespace and IPv4Namespace, which were swapped.

github-actions · 2024-09-30T07:56:43Z

🚀 Deployed on https://preview-39--hedgehog-docs.netlify.app

Fredi-raspall · 2024-09-30T08:12:28Z

I also found the definition of isolated and restricted subnets a bit confusing, esp. in some comments where it says that it does not affect VPC peering, and the relation to permit lists. Are isolated and restricted meant to be some default initial config that can be overridden by permit lists? Is the idea like in firewalls where you can deny everything and the except the accepted or allow everything and then except the stuff to be blocked? IMO either a commented example or the interaction would make it clearer... or maybe a matrix with distinct combinations.

docs/user-guide/vpcs.md

mrbojangles3

Couple of small changes

Frostman · 2024-09-30T14:48:08Z

docs/concepts/overview.md

@@ -41,8 +41,8 @@ Wiring Diagram consists of the following resources:
 ### User-facing API

 * VPC API
-    * __VPC__: Virtual Private Cloud, similar to the public cloud VPC it provides an isolated private network for the
-      resources, with support for multiple subnets, each with user-provided VLANs and on-demand DHCP
+    * __VPC__: Virtual Private Cloud, similar a the public cloud VPC, provides an isolated private network for the


"similar a" - typo?

Frostman · 2024-09-30T14:49:06Z

docs/concepts/overview.md

@@ -86,15 +86,15 @@ Switch boot and installation.
 * Actual SONiC installers
 * Miscellaneous: rsyslog/ntp

-## Fabric
+## Fabric Agent


It's "Fabric"

Yes, I know the agent is called Fabric. However, on my first read, I thought it would have been clearer to stress that it is an agent, since "fabric" could refer to the HH Fabric. Removing it ...

IMO it refers to the HH Fabric here

Does it? When I said "HH Fabric" I meant the switches + control nodes ...

Frostman · 2024-09-30T14:49:55Z

docs/concepts/overview.md


-Control plane and switch agent.
+Control-plane and switch agent.


No hyphen needed

Frostman · 2024-09-30T14:55:20Z

docs/user-guide/vpcs.md


 ```yaml
 apiVersion: wiring.githedgehog.com/v1alpha2
-kind: VLANNamespace
+kind: VLANNamespac`


Oh boy. I was hoping to fix typos on my way through the documentation.... not meaning to add new ...

Frostman · 2024-09-30T14:59:42Z

I also found the definition of isolated and restricted subnets a bit confusing, esp. in some comments where it says that it does not affect VPC peering, and the relation to permit lists. Are isolated and restricted meant to be some default initial config that can be overridden by permit lists? Is the idea like in firewalls where you can deny everything and the except the accepted or allow everything and then except the stuff to be blocked? IMO either a commented example or the interaction would make it clearer... or maybe a matrix with distinct combinations.

Yeah, isolated/resitricted/permit (in the VPC spec) are little bit tricky. Using any of them only affects access between the subnets within a VPC and doesn't affect VPC peering which has its own controls. Permit lists are "Overrides" on top of having defaultIsolated/defaultRestricted and/or isolated/restircted set on the specific subnets. Example would be setting defaultIsolated to true (no subnets in VPC can talk to each other) and explicitly listing subnets that should be able to talk in the permit list.

Fredi-raspall · 2024-09-30T15:41:31Z

I also found the definition of isolated and restricted subnets a bit confusing, esp. in some comments where it says that it does not affect VPC peering, and the relation to permit lists. Are isolated and restricted meant to be some default initial config that can be overridden by permit lists? Is the idea like in firewalls where you can deny everything and the except the accepted or allow everything and then except the stuff to be blocked? IMO either a commented example or the interaction would make it clearer... or maybe a matrix with distinct combinations.

Yeah, isolated/resitricted/permit (in the VPC spec) are little bit tricky. Using any of them only affects access between the subnets within a VPC and doesn't affect VPC peering which has its own controls. Permit lists are "Overrides" on top of having defaultIsolated/defaultRestricted and/or isolated/restircted set on the specific subnets. Example would be setting defaultIsolated to true (no subnets in VPC can talk to each other) and explicitly listing subnets that should be able to talk in the permit list.

Thanks for the explanation @Frostman . Does this mean that if subnets are defined with isolated = False, then no permit list is needed and they can freely communicate (there's no deny-list, right?).

Frostman · 2024-09-30T20:41:12Z

@Fredi-raspall correct, by default both isolated and restricted are false and so all subnets within a VPC can talk to each other as well as all hosts within every subnet can talk to each other too

Fredi-raspall · 2024-10-01T12:22:16Z

Understood @Frostman . Thanks!

Fredi-raspall · 2024-10-08T11:13:17Z

@Frostman , @mrbojangles3 I think I've gone through most of the documentation now and believe to have corrected existing minor things (plus my personal contributions ;-) ). I've also added a table to ease the access to the supported platforms. I don't plan to work more on this branch, so a quick review from either of you would be great. Thx!

Frostman · 2024-10-08T15:20:22Z

docs/reference/profiles.md

@@ -3,6 +3,19 @@
 The following is a list of all supported switches. Please, make sure to use the version of documentation that matches your environment to get an
 up-to-date list of supported switches, their features and port naming scheme.

+| Switch Platforms |


This file is generated from fabric repo, so, shouldn't be edited directly.

I'm fine removing the table. Maybe one could be auto-generated then?

I've removed it.

Yes, we can gen it, it's a good idea. It's done in https://github.com/githedgehog/fabric/blob/master/pkg/gen/profilesref/profilesref.go

Frostman · 2024-10-08T15:21:42Z

docs/user-guide/connections.md


 ## Server connections (user-facing)

-Server connections are used to connect workload servers to the switches.
+Server connections are used to connect workload (or control) servers to switches.


Those connections are only used for workload servers

I've removed it and changed section name to "Workload server connections"

Frostman · 2024-10-08T15:22:31Z

docs/concepts/overview.md

@@ -29,9 +29,9 @@ systems.

 Wiring Diagram consists of the following resources:

-* "Devices": describes any device in the Fabric
+* __Devices__: describes *any* device in the Fabric


There is no such object named Device, so I was trying to separate it somehow from Switches and Servers. What do you think? I'm ok both ways.

I've reverted it to quotes and rephrased as:

"Devices": describe any device in the Fabric and can be of two types:

@Frostman I think I've addressed all the comments and believe this is good to go.

Including: * Unified some formatting. * removed spurious ```yaml that was being rendered. * Fixed the definitions for VLANNamespace and IPv4Namespace, which were swapped.

Frostman · 2024-10-08T17:45:26Z

@Fredi-raspall thank you!

Fredi-raspall requested review from Frostman, sergeymatov, amitlimaye, sonoble, thedvorkin and mrbojangles3 as code owners September 30, 2024 07:56

github-actions bot temporarily deployed to pull request September 30, 2024 07:56 Inactive

mrbojangles3 reviewed Sep 30, 2024

View reviewed changes

docs/user-guide/vpcs.md Outdated Show resolved Hide resolved

mrbojangles3 reviewed Sep 30, 2024

View reviewed changes

docs/user-guide/vpcs.md Outdated Show resolved Hide resolved

mrbojangles3 requested changes Sep 30, 2024

View reviewed changes

Frostman requested changes Sep 30, 2024

View reviewed changes

Fredi-raspall force-pushed the fredi/minor_fixes branch from 635081a to 0d5e32d Compare September 30, 2024 15:42

github-actions bot temporarily deployed to pull request September 30, 2024 15:43 Inactive

Fredi-raspall force-pushed the fredi/minor_fixes branch from 0d5e32d to 01349f5 Compare September 30, 2024 16:14

github-actions bot temporarily deployed to pull request September 30, 2024 16:14 Inactive

Fredi-raspall force-pushed the fredi/minor_fixes branch from 01349f5 to 66be916 Compare October 8, 2024 11:07

github-actions bot temporarily deployed to pull request October 8, 2024 11:08 Inactive

mrbojangles3 previously approved these changes Oct 8, 2024

View reviewed changes

Frostman requested changes Oct 8, 2024

View reviewed changes

Misc. minor corrections.

143e173

Including: * Unified some formatting. * removed spurious ```yaml that was being rendered. * Fixed the definitions for VLANNamespace and IPv4Namespace, which were swapped.

Fredi-raspall dismissed mrbojangles3’s stale review via 143e173 October 8, 2024 17:30

Fredi-raspall force-pushed the fredi/minor_fixes branch from 66be916 to 143e173 Compare October 8, 2024 17:30

github-actions bot temporarily deployed to pull request October 8, 2024 17:31 Inactive

Frostman merged commit 4065c95 into master Oct 8, 2024
2 checks passed

Frostman deleted the fredi/minor_fixes branch October 8, 2024 17:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Misc. minor corrections, including: #39

Misc. minor corrections, including: #39

Fredi-raspall commented Sep 30, 2024

github-actions bot commented Sep 30, 2024 •

edited

Loading

Fredi-raspall commented Sep 30, 2024

mrbojangles3 left a comment

Frostman Sep 30, 2024

Frostman Sep 30, 2024

Fredi-raspall Sep 30, 2024

Frostman Sep 30, 2024

Fredi-raspall Sep 30, 2024

Frostman Sep 30, 2024

Frostman Sep 30, 2024

Fredi-raspall Sep 30, 2024

Frostman commented Sep 30, 2024

Fredi-raspall commented Sep 30, 2024

Frostman commented Sep 30, 2024 •

edited

Loading

Fredi-raspall commented Oct 1, 2024

Fredi-raspall commented Oct 8, 2024

Frostman Oct 8, 2024

Fredi-raspall Oct 8, 2024

Fredi-raspall Oct 8, 2024

Frostman Oct 8, 2024

Frostman Oct 8, 2024

Fredi-raspall Oct 8, 2024

Frostman Oct 8, 2024

Fredi-raspall Oct 8, 2024

Fredi-raspall Oct 8, 2024

Frostman commented Oct 8, 2024


		Control plane and switch agent.
		Control-plane and switch agent.

Misc. minor corrections, including: #39

Misc. minor corrections, including: #39

Conversation

Fredi-raspall commented Sep 30, 2024

github-actions bot commented Sep 30, 2024 • edited Loading

Fredi-raspall commented Sep 30, 2024

mrbojangles3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Frostman commented Sep 30, 2024

Fredi-raspall commented Sep 30, 2024

Frostman commented Sep 30, 2024 • edited Loading

Fredi-raspall commented Oct 1, 2024

Fredi-raspall commented Oct 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Frostman commented Oct 8, 2024

github-actions bot commented Sep 30, 2024 •

edited

Loading

Frostman commented Sep 30, 2024 •

edited

Loading