Introduce Snowflake backend for Theia #112

antoninbas · 2022-09-16T00:40:51Z

Snowflake can be used as the data store for Theia, and to run applications such as policy recommendation (using Snowflake virtual warehouses). Users need to bring their own Snowflake and AWS accounts.

In this first commit, we add a new CLI tool (theia-sf), which can be used to create the necessary cloud resources in Snowflake and AWS. The CLI is built using Pulumi, an "infrastructure-as-code" tool, which is itself built using Terraform providers. No third-party tool needs to be installed directly by users, as theia-sf will install dependencies itself in a temporary directory.

When running "theia-sf onboard", which will provision infrastructure, users need to:

configure AWS and Snowflake credentials
provide an S3 bucket to store infrastructure state
provide a KMS key to encrypt infrastructure state (notably secrets)

The advantage of using Pulumi to provision cloud resources is the potential ability to support upgrades easily: resources can be created, updated or deleted as needed whenever "theia-sf onboard" runs.

Pulumi is used to create a Snowflake database and schema. However, it is not used to create objects (tables, views, etc.) within that schema. For that we use a database migration tool. The rationale is that the Pulumi / Terraform provider for Snowflake doesn't support table schema updates very well, and our current solution is more flexible. Once again, this should enable us to support upgrades easily.

Signed-off-by: Antonin Bas abas@vmware.com

snowflake/README.md

snowflake/cmd/createKmsKey.go

snowflake/database/migrations/000002_create_pods_view.up.sql

snowflake/database/migrations/000002_create_pods_view.down.sql

dreamtalen

partial reviews

dreamtalen · 2022-09-19T17:36:35Z

snowflake/README.md

+to specify credentials that can be used by the AWS Go SDK. Either configure the
+~/.aws/credentials file or set the required environment variables.
+
+You can also export the `AWS_REGION` environment variable, set to your preferred


Suggested change

You can also export the `AWS_REGION` environment variable, set to your preferred

You can also export the `AWS_REGION` environment variable, and set it to your preferred

this is fine I think, "set" is also the past participle of the "set" verb

dreamtalen · 2022-09-19T17:58:56Z

snowflake/cmd/createKmsKey.go

+var createKmsKeyCmd = &cobra.Command{
+	Use:   "create-kms-key",
+	Short: "Create an AWS KMS key",
+	Long: `This command creates a new KMS key in your AWS account. They key


Suggested change

Long: `This command creates a new KMS key in your AWS account. They key

Long: `This command creates a new KMS key in your AWS account. This key

dreamtalen · 2022-09-19T18:42:31Z

snowflake/cmd/offboard.go

+	offboardCmd.Flags().String("bucket-prefix", "antrea-flows-infra", "prefix to use to store infra state")
+	offboardCmd.Flags().String("bucket-region", "", "region where infra bucket is defined; if omitted, we will try to get the region from AWS")
+	offboardCmd.Flags().String("key-id", GetEnv("THEIA_SF_KMS_KEY_ID", ""), "Kms Key ID")
+	offboardCmd.Flags().String("key-region", "", "Kms Key region")


Suggested change

offboardCmd.Flags().String("key-region", "", "Kms Key region")

offboardCmd.Flags().String("key-region", "", "Kms key region")

to be consistent with elsewhere

dreamtalen · 2022-09-19T18:42:52Z

snowflake/cmd/onboard.go

+	onboardCmd.Flags().String("bucket-prefix", "antrea-flows-infra", "prefix to use to store infra state")
+	onboardCmd.Flags().String("bucket-region", "", "region where infra bucket is defined; if omitted, we will try to get the region from AWS")
+	onboardCmd.Flags().String("key-id", GetEnv("THEIA_SF_KMS_KEY_ID", ""), "Kms key ID")
+	onboardCmd.Flags().String("key-region", "", "Kms Key region")


Suggested change

onboardCmd.Flags().String("key-region", "", "Kms Key region")

onboardCmd.Flags().String("key-region", "", "Kms key region")

to be consistent with elsewhere

dreamtalen · 2022-09-19T18:45:35Z

snowflake/cmd/receiveSqsMessage.go

+Snowflake account as expected.
+
+To receive a message without deleting it (message will remain in the queue and
+become available to consummers again after a short time interval):


Suggested change

become available to consummers again after a short time interval):

become available to consumers again after a short time interval):

dreamtalen · 2022-09-19T18:54:19Z

snowflake/cmd/receiveSqsMessage.go

+	if len(output.Messages) == 0 {
+		return nil
+	}
+	message := output.Messages[0]


Looks like we could only see the top message using this command. Shall we have an option that could display all messages?

Honestly even though this is not difficult to do, I am not sure this is needed. We can see if this is requested in the future.

heanlan · 2022-09-27T02:06:44Z

for dashboard needs, can we add a few fields to the pods view:

CREATE VIEW IF NOT EXISTS pods(
  flowStartSeconds,
  flowEndSeconds,
  packetDeltaCount,
  octetDeltaCount,
  reversePacketDeltaCount,
  reverseOctetDeltaCount,
  sourcePodName,
  sourcePodNamespace,
  source,
  destinationPodName,
  destinationPodNamespace,
  destination,
  throughput,
  reverseThroughput,
  clusterUUID,
  flowType,
  timeInserted
) as SELECT
  flowStartSeconds,
  flowEndSeconds,
  packetDeltaCount,
  octetDeltaCount,
  reversePacketDeltaCount,
  reverseOctetDeltaCount,
  sourcePodName,
  sourcePodNamespace,
  sourcePodNamespace || '/' || sourcePodName || ':' || SOURCETRANSPORTPORT,
  destinationPodName,
  destinationPodNamespace,
  destinationPodNamespace || '/' || destinationPodName|| ':' || DESTINATIONTRANSPORTPORT,
  throughput,
  reverseThroughput,
  flowtype,
  timeInserted
 FROM flows

and add another policies view:

CREATE VIEW IF NOT EXISTS policies 
as SELECT
 flowEndSeconds,
 octetDeltaCount,
 reverseOctetDeltaCount,
 egressNetworkPolicyName,
 egressNetworkPolicyNamespace,
 egressNetworkPolicyRuleAction,
 ingressNetworkPolicyName,
 ingressNetworkPolicyNamespace,
 ingressNetworkPolicyRuleAction,
 sourcePodName,
 sourcePodNamespace,
 sourceTransportPort,
 destinationIP,
 destinationPodName,
 destinationPodNamespace,
 destinationTransportPort,
 destinationServicePortName,
 destinationServicePort,
 throughput,
 flowtype
FROM flows

thanks

antoninbas · 2022-09-27T04:49:01Z

@heanlan I need a clarification for one of the changes to the pods view. Is it necessary to add the source transport port to the source field? I am asking because usually information about the source port is not very important. Is this used anywhere in the dashboards? And more generally, does it make sense to concatenate the port number with namespace + name, or should we keep it as a separate field?

heanlan · 2022-09-27T16:46:56Z

I need a clarification for one of the changes to the pods view. Is it necessary to add the source transport port to the source field? I am asking because usually information about the source port is not very important. Is this used anywhere in the dashboards? And more generally, does it make sense to concatenate the port number with namespace + name, or should we keep it as a separate field?

In ClickHouse dashboards, we use sourcePodNamespace/sourcePodName:port -> destinationPodNamespace/destinationPodName:port to identify a pod-to-pod connection. Because sometimes a sourcePod restarts, and its port changes. We think they should be considered as two different connections before&after the restart.

In ClickHouse, the schema of view does not concatenate any fields. We do the concatenation in the queries. Because we use the same view in pod-to-pod, pod-to-service, pod-to-external dashboards. The fields need to be concatenated are be different according to the dashboards. Currently in snowflake, I didn't build pod-to-service, pod-to-external dashboards, and I saw you already have created concatenated fields source and destination, so I was thinking maybe it's ok to also concatenate port.

antoninbas · 2022-09-27T19:04:24Z

@heanlan if this is only useful for one dashboard and you can do it in the query, it may not make sense to concatenate the port in the view at the moment. I concatenate namespace + name in the view because it is useful for filtering IMO, independently of the dashboard.

antoninbas · 2022-09-27T19:08:31Z

@heanlan could you review the SQL migrations changes

I kept the clusterUUID in the pods view (you had removed it in your comment)
I added the clusterUUID to the policies view

Could you also confirm that you don't need timeInserted in the policies view?

Let me know if you think we can remove timeInserted from the pods view as well. Maybe we don't need it, I am not sure why I added it in the first place.

snowflake/database/migrations/000002_create_pods_view.up.sql

This PR adds 4 Grafana dashboards with snowflake as the datasource: Home Dashboard, Flow Records Dashboard, Pod-to-Pod Flows Dashboard, and Network-Policy Flows Dashboard. These dashboards are basically the same with ClickHouse dashboards in their contents, differ in the query syntax. Currently, the snowflake dashboards are without the ad-hoc filters, as the datasource plugin does not offer the the support. We can revisit it later if we decide to work with Grafana in the long-term. In the deployment instruction, we suggest users deploy Grafana with docker, so they do not need to install Grafana locally, and it makes the custom plugins installation simple. This PR should be merged after antrea-io#112 and antrea-io#118. The documentation shall also be merged into the README.md introduced in antrea-io#112 Signed-off-by: heanlan <hanlan@vmware.com>

heanlan · 2022-09-28T18:19:48Z

Sorry, I missed this one.

Could you also confirm that you don't need timeInserted in the policies view?

don't need timeInserted in policies

Let me know if you think we can remove timeInserted from the pods view as well. Maybe we don't need it, I am not sure why I added it in the first place.

don't need timeInserted in pods

antoninbas · 2022-09-28T18:56:34Z

@heanlan thanks for confirming, I removed timeInserted from the pods view

snowflake/database/migrations/000002_create_pods_view.up.sql

snowflake/database/migrations/000003_create_policies_view.up.sql

heanlan

Sorry for adding comments incrementally. I believe it should be the last comment on the schema part.

snowflake/database/migrations/000002_create_pods_view.up.sql

antoninbas · 2022-09-30T23:44:34Z

Sorry for adding comments incrementally. I believe it should be the last comment on the schema part.

no worries, it's my bad for having these typos
I'm planning to run a new round of e2e tests after we go through review and before merging

dreamtalen

LGTM

heanlan

LGTM

snowflake/main.go

snowflake/cmd/root.go

Snowflake can be used as the data store for Theia, and to run applications such as policy recommendation (using Snowflake virtual warehouses). Users need to bring their own Snowflake and AWS accounts. In this first commit, we add a new CLI tool (theia-sf), which can be used to create the necessary cloud resources in Snowflake and AWS. The CLI is built using Pulumi, an "infrastructure-as-code" tool, which is itself built using Terraform providers. No third-party tool needs to be installed directly by users, as theia-sf will install dependencies itself in a temporary directory. When running "theia-sf onboard", which will provision infrastructure, users need to: * configure AWS and Snowflake credentials * provide an S3 bucket to store infrastructure state * provide a KMS key to encrypt infrastructure state (notably secrets) The advantage of using Pulumi to provision cloud resources is the potential ability to support upgrades easily: resources can be created, updated or deleted as needed whenever "theia-sf onboard" runs. Pulumi is used to create a Snowflake database and schema. However, it is not used to create objects (tables, views, etc.) within that schema. For that we use a database migration tool. The rationale is that the Pulumi / Terraform provider for Snowflake doesn't support table schema updates very well, and our current solution is more flexible. Once again, this should enable us to support upgrades easily. Signed-off-by: Antonin Bas <abas@vmware.com>

Signed-off-by: Antonin Bas <abas@vmware.com>

antoninbas · 2022-10-04T18:43:10Z

/theia-test-e2e

Signed-off-by: Antonin Bas <abas@vmware.com>

antoninbas · 2022-10-04T18:50:44Z

/theia-test-e2e

This PR adds 4 Grafana dashboards with snowflake as the datasource: Home Dashboard, Flow Records Dashboard, Pod-to-Pod Flows Dashboard, and Network-Policy Flows Dashboard. These dashboards are basically the same with ClickHouse dashboards in their contents, differ in the query syntax. Currently, the snowflake dashboards are without the ad-hoc filters, as the datasource plugin does not offer the the support. We can revisit it later if we decide to work with Grafana in the long-term. In the deployment instruction, we suggest users deploy Grafana with docker, so they do not need to install Grafana locally, and it makes the custom plugins installation simple. This PR should be merged after antrea-io#112 and antrea-io#118. The documentation shall also be merged into the README.md introduced in antrea-io#112 Signed-off-by: heanlan <hanlan@vmware.com>

This PR adds 4 Grafana dashboards with snowflake as the datasource: Home Dashboard, Flow Records Dashboard, Pod-to-Pod Flows Dashboard, and Network-Policy Flows Dashboard. These dashboards are basically the same with ClickHouse dashboards in their contents, differ in the query syntax. Currently, the snowflake dashboards are without the ad-hoc filters, as the datasource plugin does not offer the the support. We can revisit it later if we decide to work with Grafana in the long-term. In the deployment instruction, we suggest users deploy Grafana with docker, so they do not need to install Grafana locally, and it makes the custom plugins installation simple. This PR should be merged after #112 and #118. The documentation shall also be merged into the README.md introduced in #112 Signed-off-by: heanlan <hanlan@vmware.com>

antoninbas requested review from heanlan, dreamtalen and salv-orlando September 16, 2022 00:41

antoninbas added this to the Theia v0.3 Release milestone Sep 16, 2022

heanlan reviewed Sep 16, 2022

View reviewed changes

dreamtalen reviewed Sep 19, 2022

View reviewed changes

heanlan reviewed Sep 27, 2022

View reviewed changes

snowflake/database/migrations/000002_create_pods_view.up.sql Show resolved Hide resolved

heanlan mentioned this pull request Sep 28, 2022

Add Grafana dashboards with snowflake #119

Merged

antoninbas force-pushed the snowflake branch from 3304f8f to aa20659 Compare September 28, 2022 18:47

heanlan reviewed Sep 29, 2022

View reviewed changes

snowflake/database/migrations/000002_create_pods_view.up.sql Outdated Show resolved Hide resolved

snowflake/database/migrations/000003_create_policies_view.up.sql Outdated Show resolved Hide resolved

antoninbas force-pushed the snowflake branch from aa20659 to f550077 Compare September 30, 2022 18:20

heanlan reviewed Sep 30, 2022

View reviewed changes

snowflake/database/migrations/000002_create_pods_view.up.sql Outdated Show resolved Hide resolved

antoninbas force-pushed the snowflake branch from f550077 to dadddb7 Compare September 30, 2022 23:43

dreamtalen approved these changes Oct 3, 2022

View reviewed changes

heanlan approved these changes Oct 4, 2022

View reviewed changes

snowflake/main.go Outdated Show resolved Hide resolved

snowflake/cmd/root.go Outdated Show resolved Hide resolved

antoninbas added 6 commits October 4, 2022 11:42

Address review comments

52d3390

Signed-off-by: Antonin Bas <abas@vmware.com>

Add copyright notice to new Go source files

cdc7bf1

Signed-off-by: Antonin Bas <abas@vmware.com>

Address review comments

620c3b8

Signed-off-by: Antonin Bas <abas@vmware.com>

Update to SQL migrations

5d882fd

Signed-off-by: Antonin Bas <abas@vmware.com>

Add transport ports to pods view

19c7020

Signed-off-by: Antonin Bas <abas@vmware.com>

antoninbas added 3 commits October 4, 2022 11:42

Remove timeInserted from pods view

8a8d0e5

Signed-off-by: Antonin Bas <abas@vmware.com>

Fix SQL migrations

5987af9

Signed-off-by: Antonin Bas <abas@vmware.com>

Add missing copyright headers

cecea6d

Signed-off-by: Antonin Bas <abas@vmware.com>

antoninbas force-pushed the snowflake branch from dadddb7 to cecea6d Compare October 4, 2022 18:42

s/bucketName=flows/bucketPrefix=flows/ in Helm install

9d84c7c

Signed-off-by: Antonin Bas <abas@vmware.com>

antoninbas merged commit c051c90 into antrea-io:main Oct 4, 2022

antoninbas deleted the snowflake branch October 4, 2022 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Snowflake backend for Theia #112

Introduce Snowflake backend for Theia #112

antoninbas commented Sep 16, 2022

dreamtalen left a comment

dreamtalen Sep 19, 2022

antoninbas Sep 19, 2022

dreamtalen Sep 19, 2022

dreamtalen Sep 19, 2022

dreamtalen Sep 19, 2022

dreamtalen Sep 19, 2022

dreamtalen Sep 19, 2022

antoninbas Sep 19, 2022

heanlan commented Sep 27, 2022

antoninbas commented Sep 27, 2022

heanlan commented Sep 27, 2022

antoninbas commented Sep 27, 2022

antoninbas commented Sep 27, 2022 •

edited

Loading

heanlan commented Sep 28, 2022

antoninbas commented Sep 28, 2022

heanlan left a comment

antoninbas commented Sep 30, 2022

dreamtalen left a comment

heanlan left a comment

antoninbas commented Oct 4, 2022

antoninbas commented Oct 4, 2022

	You can also export the `AWS_REGION` environment variable, set to your preferred
	You can also export the `AWS_REGION` environment variable, and set it to your preferred

	Long: `This command creates a new KMS key in your AWS account. They key
	Long: `This command creates a new KMS key in your AWS account. This key

	offboardCmd.Flags().String("key-region", "", "Kms Key region")
	offboardCmd.Flags().String("key-region", "", "Kms key region")

	onboardCmd.Flags().String("key-region", "", "Kms Key region")
	onboardCmd.Flags().String("key-region", "", "Kms key region")

	become available to consummers again after a short time interval):
	become available to consumers again after a short time interval):

Introduce Snowflake backend for Theia #112

Introduce Snowflake backend for Theia #112

Conversation

antoninbas commented Sep 16, 2022

dreamtalen 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

heanlan commented Sep 27, 2022

antoninbas commented Sep 27, 2022

heanlan commented Sep 27, 2022

antoninbas commented Sep 27, 2022

antoninbas commented Sep 27, 2022 • edited Loading

heanlan commented Sep 28, 2022

antoninbas commented Sep 28, 2022

heanlan left a comment

Choose a reason for hiding this comment

antoninbas commented Sep 30, 2022

dreamtalen left a comment

Choose a reason for hiding this comment

heanlan left a comment

Choose a reason for hiding this comment

antoninbas commented Oct 4, 2022

antoninbas commented Oct 4, 2022

antoninbas commented Sep 27, 2022 •

edited

Loading