separate Istio installation config from other Istio-related config

tcdowney commented 4 years ago

Summary of changes

Previously we were including all Istio-related config as part of the xxx-generated-istio.yaml file making it unclear which portions of it were actually part of our istioctl manifest generate step.

This was confusing for our downstream consumers and tightly coupled us to how we were installing Istio.

This change leaves the config/istio directory as the home for ytt overlays and istioctl manifest values for generating the installation YAML. The generated YAML has now been moved from istio-generated to istio/generated to consolidate all installation artifacts into a single directory.

Other Istio-related config like our NetworkPolicies for the Istio control plane and workloads namespace Sidecar config has been moved into a separate istio-config directory. This is a prefactor for our Istio 1.6 upgrade story (#172031795) and I'd like to get some additional eyes on it since I'm soloing today.

Additional Context

We can add the new directories to the cf-for-k8s vendir.yml file before we release and then remove config/istio-generated when we bump to our next release.

$ git diff vendir.yml
diff --git a/vendir.yml b/vendir.yml
index ef5d29c..fb33e1b 100644
--- a/vendir.yml
+++ b/vendir.yml
@@ -14,6 +14,8 @@ directories:
     - cfroutesync/crds/**/*
     - config/cfroutesync/**/*
     - config/crd/**/*
+    - config/istio-config/**/*
+    - config/istio/generated/**/*
     - config/istio-generated/**/*
   - path: github.com/cloudfoundry/capi-k8s-release
     git:

Acceptance Steps

Check out these changes, make the above changes to the vendir.yml in cf-for-k8s then run the vendir sync command for your local changes (or point the vendir.yml to this commit SHA):

vendir sync --directory config/_ytt_lib/github.com/cloudfoundry/cf-k8s-networking=/home/pivotal/workspace/cf-k8s-networking

Deploy cf-for-k8s. If it works and the NetworkPolicy in the istio-system namespace / Sidecar in cf-workloads still looks good, we're good.

cc/ @ndhanushkodi @christianang @jenspinney @XanderStrike

cf-gitbot commented 4 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/173204175

The labels on this github issue will be updated when the story is started.

rosenhouse commented 4 years ago

@flawedmatrix @cppforlife

tcdowney commented 4 years ago

Talked with @christianang on Slack the other day about the chicken-and-egg problem with cf-for-k8s that this change introduced. Decided to keep the all of the installation config in the istio folder and introduce istio-config to house the other configuration. This way we can update cf-for-k8s to have the additional folders before we cut a release so that we don't jam up our pipelines. vendir sync will just ignore the directories when they're empty.

$ git diff vendir.yml
diff --git a/vendir.yml b/vendir.yml
index ef5d29c..fb33e1b 100644
--- a/vendir.yml
+++ b/vendir.yml
@@ -14,6 +14,8 @@ directories:
     - cfroutesync/crds/**/*
     - config/cfroutesync/**/*
     - config/crd/**/*
+    - config/istio-config/**/*
+    - config/istio/generated/**/*
     - config/istio-generated/**/*
   - path: github.com/cloudfoundry/capi-k8s-release
     git:

I still believe that moving config/istio-generated/ to config/istio/generated/ is good though (even though we'll have to mention both in vendir.yml until we cut our next release) since it scopes all of the installation stuff to a single directory.

cf-gitbot commented 4 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/173230717

The labels on this github issue will be updated when the story is started.

kauana commented 4 years ago

Hey @tcdowney, just letting you know we merged your changes to master already so I am closing this PR.

cc @ndhanushkodi

cloudfoundry / cf-k8s-networking