Skip to content

Commit

Permalink
provided an ability to create additional endpoints (#284)
Browse files Browse the repository at this point in the history
  • Loading branch information
shriramsharma committed Feb 24, 2023
1 parent 3ff6232 commit cabf266
Show file tree
Hide file tree
Showing 7 changed files with 576 additions and 5 deletions.
2 changes: 2 additions & 0 deletions admiral/cmd/admiral/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ func GetRootCmd(args []string) *cobra.Command {
"If Routing Policy feature needs to be enabled")
rootCmd.PersistentFlags().StringArrayVar(&params.ExcludedIdentityList, "excluded_identity_list", []string{},
"List of identities which should be excluded from getting processed")
rootCmd.PersistentFlags().StringArrayVar(&params.AdditionalEndpointSuffixes, "additional_endpoint_suffixes", []string{},
"Suffixes that Admiral should use to generate additional endpoints through VirtualServices")
return rootCmd
}

Expand Down
14 changes: 14 additions & 0 deletions admiral/pkg/clusters/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,20 @@ func getServiceEntryDiff(new *v1alpha3.ServiceEntry, old *v1alpha3.ServiceEntry)
return destructive, diff
}

func deleteVirtualService(ctx context.Context, exist *v1alpha3.VirtualService, namespace string, rc *RemoteController) error {
if exist == nil {
return fmt.Errorf("the VirtualService passed was nil")
}
err := rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(namespace).Delete(ctx, exist.Name, metav1.DeleteOptions{})
if err != nil {
if k8sErrors.IsNotFound(err) {
return fmt.Errorf("either VirtualService was already deleted, or it never existed")
}
return err
}
return nil
}

func deleteServiceEntry(ctx context.Context, exist *v1alpha3.ServiceEntry, namespace string, rc *RemoteController) {
if exist != nil {
err := rc.ServiceEntryController.IstioClient.NetworkingV1alpha3().ServiceEntries(namespace).Delete(ctx, exist.Name, metav1.DeleteOptions{})
Expand Down
79 changes: 79 additions & 0 deletions admiral/pkg/clusters/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package clusters

import (
"context"
"fmt"
"reflect"
"strings"
"testing"
Expand All @@ -12,6 +13,7 @@ import (
"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
"github.com/istio-ecosystem/admiral/admiral/pkg/controller/istio"
"github.com/istio-ecosystem/admiral/admiral/pkg/test"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"

argo "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/golang/protobuf/ptypes/duration"
Expand Down Expand Up @@ -1848,3 +1850,80 @@ func TestValidateServiceEntryEndpoints(t *testing.T) {
})
}
}

func TestDeleteVirtualService(t *testing.T) {

ctx := context.Background()
namespace := "testns"

fooVS := &v1alpha32.VirtualService{
ObjectMeta: metaV1.ObjectMeta{
Name: "stage.test00.foo-vs",
},
Spec: v1alpha3.VirtualService{
Hosts: []string{"stage.test00.foo", "stage.test00.bar"},
},
}

validIstioClient := istioFake.NewSimpleClientset()
validIstioClient.NetworkingV1alpha3().VirtualServices(namespace).Create(ctx, fooVS, metaV1.CreateOptions{})

testcases := []struct {
name string
virtualService *v1alpha32.VirtualService
rc *RemoteController
expectedError error
expectedDeletedVSName string
}{
{
name: "Given virtualservice to delete, when nil VS is passed, the func should return an error",
virtualService: nil,
expectedError: fmt.Errorf("the VirtualService passed was nil"),
},
{
name: "Given virtualservice to delete, when VS passed does not exists, the func should return an error",
virtualService: &v1alpha32.VirtualService{ObjectMeta: metaV1.ObjectMeta{Name: "vs-does-not-exists"}},
expectedError: fmt.Errorf("either VirtualService was already deleted, or it never existed"),
rc: &RemoteController{
VirtualServiceController: &istio.VirtualServiceController{
IstioClient: validIstioClient,
},
},
},
{
name: "Given virtualservice to delete, when VS exists, the func should delete the VS and not return any error",
virtualService: fooVS,
expectedError: nil,
rc: &RemoteController{
VirtualServiceController: &istio.VirtualServiceController{
IstioClient: validIstioClient,
},
},
expectedDeletedVSName: "stage.test00.foo-vs",
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {

err := deleteVirtualService(ctx, tc.virtualService, namespace, tc.rc)

if err != nil && tc.expectedError != nil {
if !strings.Contains(err.Error(), tc.expectedError.Error()) {
t.Errorf("expected %s, got %s", tc.expectedError.Error(), err.Error())
}
} else if err != tc.expectedError {
t.Errorf("expected %v, got %v", tc.expectedError, err)
}

if err == nil && tc.expectedDeletedVSName != "" {
_, err := tc.rc.VirtualServiceController.IstioClient.NetworkingV1alpha3().VirtualServices(namespace).Get(context.Background(), tc.expectedDeletedVSName, metaV1.GetOptions{})
if err != nil && !k8sErrors.IsNotFound(err) {
t.Errorf("test failed as VS should have been deleted. error: %v", err)
}
}

})
}

}
179 changes: 174 additions & 5 deletions admiral/pkg/clusters/serviceentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ import (
"strings"
"time"

v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
"gopkg.in/yaml.v2"
k8errors "k8s.io/apimachinery/pkg/api/errors"

argo "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
log "github.com/sirupsen/logrus"
"gopkg.in/yaml.v2"
networking "istio.io/api/networking/v1alpha3"
"istio.io/client-go/pkg/apis/networking/v1alpha3"
k8sAppsV1 "k8s.io/api/apps/v1"
k8sV1 "k8s.io/api/core/v1"
k8errors "k8s.io/apimachinery/pkg/api/errors"
v12 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"

"github.com/istio-ecosystem/admiral/admiral/pkg/controller/admiral"
"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
Expand Down Expand Up @@ -304,12 +304,33 @@ func generateProxyVirtualServiceForDependencies(ctx context.Context, remoteRegis
if err != nil {
return fmt.Errorf("failed generating proxy VirtualService %s due to error: %w", v.Name, err)
}
log.Infof("successfully generated proxy VirtualService %s", v.Name)
}
}
return nil
}

func getAdmiralGeneratedVirtualService(ctx context.Context, remoteController *RemoteController, listOptions v12.ListOptions,
namespace string) (*v1alpha3.VirtualService, error) {
existingVSList, err := remoteController.VirtualServiceController.IstioClient.NetworkingV1alpha3().
VirtualServices(namespace).List(ctx, listOptions)
if err != nil {
return nil, err
}
if existingVSList == nil {
return nil, fmt.Errorf("error fetching virtualservice with labels %s", listOptions.LabelSelector)
}
if len(existingVSList.Items) == 0 {
return nil, fmt.Errorf("no virtualservice found with labels %s", listOptions.LabelSelector)
}
var result *v1alpha3.VirtualService
for _, existingVS := range existingVSList.Items {
if isGeneratedByAdmiral(existingVS.Annotations) {
result = existingVS
}
}
return result, nil
}

//Does two things;
//i) Picks the GTP that was created most recently from the passed in GTP list based on GTP priority label (GTPs from all clusters)
//ii) Updates the global GTP cache with the selected GTP in i)
Expand Down Expand Up @@ -564,6 +585,15 @@ func AddServiceEntriesWithDr(ctx context.Context, rr *RemoteRegistry, sourceClus
if !skipSEUpdate {
deleteServiceEntry(ctx, oldServiceEntry, syncNamespace, rc)
cache.SeClusterCache.Delete(seDr.ServiceEntry.Hosts[0])

// Delete additional endpoints if any
if isAdditionalEndpointsEnabled() {
err := deleteAdditionalEndpoints(ctx, rc, identityId, env, syncNamespace)
if err != nil {
log.Error(err)
}
}

}
if !skipDRUpdate {
// after deleting the service entry, destination rule also need to be deleted if the service entry host no longer exists
Expand All @@ -589,6 +619,14 @@ func AddServiceEntriesWithDr(ctx context.Context, rr *RemoteRegistry, sourceClus
}
addUpdateServiceEntry(ctx, newServiceEntry, oldServiceEntry, syncNamespace, rc)
cache.SeClusterCache.Put(newServiceEntry.Spec.Hosts[0], rc.ClusterID, rc.ClusterID)

// Create additional endpoints if necessary
if isAdditionalEndpointsEnabled() {
err := createAdditionalEndpoints(ctx, rc, identityId, env, newServiceEntry.Spec.Hosts[0], syncNamespace)
if err != nil {
log.Error(err)
}
}
}
}

Expand All @@ -604,6 +642,137 @@ func AddServiceEntriesWithDr(ctx context.Context, rr *RemoteRegistry, sourceClus
}
}

func isAdditionalEndpointsEnabled() bool {
additionalEndpointSuffixes := common.GetAdditionalEndpointSuffixes()
if len(additionalEndpointSuffixes) <= 0 {
log.Debugf("no additional endpoints configured")
return false
}
return true
}

func validateAdditionalEndpointParams(identity, env string) error {
if identity == "" {
return fmt.Errorf("identity passed is empty")
}
if env == "" {
return fmt.Errorf("env passed is empty")
}
return nil
}

func getVirtualServiceListOptions(identity, env string) (v12.ListOptions, error) {
vsLabels := map[string]string{
common.GetWorkloadIdentifier(): identity,
common.GetEnvKey(): env,
}
labelSelector, err := labels.ValidatedSelectorFromSet(vsLabels)
if err != nil {
return v12.ListOptions{}, err
}
listOptions := v12.ListOptions{
LabelSelector: labelSelector.String(),
}
return listOptions, nil
}

// deleteAdditionalEndpoints deletes all the additional endpoints that were generated for this
// ServiceEntry.
func deleteAdditionalEndpoints(ctx context.Context, rc *RemoteController, identity, env, namespace string) error {

err := validateAdditionalEndpointParams(identity, env)
if err != nil {
return fmt.Errorf("failed deleting additional endpoints due to error %w", err)
}

listOptions, err := getVirtualServiceListOptions(identity, env)
if err != nil {
return fmt.Errorf("failed deleting additional endpoints due to error %w", err)
}
vsToDelete, err := getAdmiralGeneratedVirtualService(ctx, rc, listOptions, namespace)
if err != nil {
return err
}

if vsToDelete == nil {
log.Debug("skipped additional endpoints cleanup as no virtualservice was found to delete")
return nil
}

err = deleteVirtualService(ctx, vsToDelete, namespace, rc)
if err != nil {
log.Infof(LogErrFormat, "Delete", "VirtualService", vsToDelete.Name, rc.ClusterID, err)
return err
}
log.Infof(LogFormat, "Delete", "VirtualService", vsToDelete.Name, rc.ClusterID, "Success")
return nil
}

// createAdditionalEndpoints creates additional endpoints of service defined in the ServiceEntry.
// The list suffixes defined in admiralparams.AdditionalEndpointSuffixes will used to generate the endpoints
func createAdditionalEndpoints(ctx context.Context, rc *RemoteController, identity, env, destinationHostName, namespace string) error {

additionalEndpointSuffixes := common.GetAdditionalEndpointSuffixes()

err := validateAdditionalEndpointParams(identity, env)
if err != nil {
return fmt.Errorf("ailed generating additional endpoints due to error %w", err)
}

listOptions, err := getVirtualServiceListOptions(identity, env)
if err != nil {
return fmt.Errorf("failed generating additional endpoints due to error %w", err)
}
existingVS, err := getAdmiralGeneratedVirtualService(ctx, rc, listOptions, namespace)
if err != nil {
log.Warn(err.Error())
}

virtualServiceHostnames := make([]string, 0)
for _, suffix := range additionalEndpointSuffixes {
hostName := common.GetCnameVal([]string{env, identity, suffix})
virtualServiceHostnames = append(virtualServiceHostnames, hostName)
}
if len(virtualServiceHostnames) == 0 {
return fmt.Errorf("failed generating additional endpoints for suffixes %s", additionalEndpointSuffixes)
}

vsRoutes := []*networking.HTTPRouteDestination{
{
Destination: &networking.Destination{
Host: destinationHostName,
Port: &networking.PortSelector{
Number: common.DefaultServiceEntryPort,
},
},
},
}
vs := networking.VirtualService{
Hosts: virtualServiceHostnames,
Http: []*networking.HTTPRoute{
{
Route: vsRoutes,
},
},
}

defaultVSName := getIstioResourceName(virtualServiceHostnames[0], "-vs")
//nolint
virtualService := createVirtualServiceSkeleton(vs, defaultVSName, namespace)
// Add labels and create/update VS
vsLabels := map[string]string{
common.GetWorkloadIdentifier(): identity,
common.GetEnvKey(): env,
}
virtualService.Labels = vsLabels
err = addUpdateVirtualService(ctx, virtualService, existingVS, namespace, rc)
if err != nil {
return fmt.Errorf("failed generating additional endpoints from serviceentry due to error: %w", err)
}

return nil
}

func isGeneratedByAdmiral(annotations map[string]string) bool {
seAnnotationVal, ok := annotations[resourceCreatedByAnnotationLabel]
if !ok || seAnnotationVal != resourceCreatedByAnnotationValue {
Expand Down
Loading

0 comments on commit cabf266

Please sign in to comment.