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.

Sign up for GitHub

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

Ingress status #1976

Closed
wants to merge 2 commits into from
Closed

Ingress status #1976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a65d9b6
Implement Status for Ingress objects
stevesloka Nov 25, 2019
952c085
Add Ingress docs section and document flags to pass to Contour for
stevesloka Dec 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/contour/certgen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func GenerateCerts(certConfig *certgenConfig) (map[string][]byte, error) {

// OutputCerts outputs the certs in certs as directed by config.
func OutputCerts(config *certgenConfig,
kubeclient *kubernetes.Clientset,
kubeclient kubernetes.Interface,
certs map[string][]byte) {

if config.OutputPEM {
Expand Down
13 changes: 12 additions & 1 deletion cmd/contour/contour.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,17 @@ func main() {
}
}

func newClient(kubeconfig string, inCluster bool) (*kubernetes.Clientset, *clientset.Clientset, *coordinationv1.CoordinationV1Client) {
// Returns the current namespace in which the controller is running
// If error returns empty string
Copy link
Contributor

Choose a reason for hiding this comment

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

s/returns the empty string/returns "projectcontour"

func getCurrentNamespace() string {
ns, _, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(clientcmd.NewDefaultClientConfigLoadingRules(), &clientcmd.ConfigOverrides{}).Namespace()
Copy link
Contributor

Choose a reason for hiding this comment

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

wow, that's a function name and a half :)

How does this determine where its loading the kubeconfig from? If its magic, can we use the same magic for contour's --kubeconfig flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's using client-go to determine which namespace Contour is deployed into. I was trying to avoid folks having to add extra args to deployments by automatically finding this information.

It can still be overridden by args if the user would like. I also didn't want to read the /var/run/secrets/kubernetes.io/serviceaccount/namespace path in the pod which seems like hack when we have a k8s client (via client-go).

if err != nil {
return "projectcontour"
}
return ns
}

func newClient(kubeconfig string, inCluster bool) (kubernetes.Interface, *clientset.Clientset, *coordinationv1.CoordinationV1Client) {
var err error
var config *rest.Config
if kubeconfig != "" && !inCluster {
Expand All @@ -112,6 +122,7 @@ func newClient(kubeconfig string, inCluster bool) (*kubernetes.Clientset, *clien

client, err := kubernetes.NewForConfig(config)
check(err)

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe use \n on line 127 and onwards for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry I don't follow your comment. I didn't change any of this short of moving from kubernetes.ClientSet to kubernetes.Interface. The check(err) stops execution of the program.

contourClient, err := clientset.NewForConfig(config)
check(err)
coordinationClient, err := coordinationv1.NewForConfig(config)
Expand Down
4 changes: 2 additions & 2 deletions cmd/contour/leadership.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

// newLeaderElector creates a new leaderelection.LeaderElector and associated
// channels by which to observe elections and depositions.
func newLeaderElector(log logrus.FieldLogger, ctx *serveContext, client *kubernetes.Clientset, coordinationClient *coordinationv1.CoordinationV1Client) (*leaderelection.LeaderElector, chan struct{}, chan struct{}) {
func newLeaderElector(log logrus.FieldLogger, ctx *serveContext, client kubernetes.Interface, coordinationClient *coordinationv1.CoordinationV1Client) (*leaderelection.LeaderElector, chan struct{}, chan struct{}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like an unrelated change, maybe pull it out into its own PR to make this easier to review.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to add a test which actually tested that I could set the status of an Ingress object. In doing that I can use the client-go fake objects which gives me a fake k8s api. In order to use this I need to pass around the interface which I changed up in (https://github.com/projectcontour/contour/pull/1976/files/952c08508fffd3c71a7c0030f6a8e41511a3ba13#diff-b62d9e38d59686d41f8f41098ff436f9R112) to return an interface. That change bubbles down through all the other methods.


// leaderOK will block gRPC startup until it's closed.
leaderOK := make(chan struct{})
Expand Down Expand Up @@ -64,7 +64,7 @@ func newLeaderElector(log logrus.FieldLogger, ctx *serveContext, client *kuberne

// newResourceLock creates a new resourcelock.Interface based on the Pod's name,
// or a uuid if the name cannot be determined.
func newResourceLock(ctx *serveContext, client *kubernetes.Clientset, coordinationClient *coordinationv1.CoordinationV1Client) resourcelock.Interface {
func newResourceLock(ctx *serveContext, client kubernetes.Interface, coordinationClient *coordinationv1.CoordinationV1Client) resourcelock.Interface {
resourceLockID, found := os.LookupEnv("POD_NAME")
if !found {
resourceLockID = uuid.New().String()
Expand Down
13 changes: 11 additions & 2 deletions cmd/contour/serve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ func registerServe(app *kingpin.Application) (*kingpin.CmdClause, *serveContext)
serve.Flag("root-namespaces", "Restrict contour to searching these namespaces for root ingress routes").StringVar(&ctx.rootNamespaces)

serve.Flag("ingress-class-name", "Contour IngressClass name").StringVar(&ctx.ingressClass)
serve.Flag("envoy-service-name", "Defines which Envoy service name handles ingress traffic for this instance of Contour").StringVar(&ctx.envoyServiceName)
serve.Flag("envoy-service-namespace", "Defines which Envoy service namespace handles ingress traffic for this instance of Contour").StringVar(&ctx.envoyServiceNamespace)

serve.Flag("envoy-http-access-log", "Envoy HTTP access log").StringVar(&ctx.httpAccessLog)
serve.Flag("envoy-https-access-log", "Envoy HTTPS access log").StringVar(&ctx.httpsAccessLog)
Expand All @@ -125,6 +127,11 @@ func doServe(log logrus.FieldLogger, ctx *serveContext) error {
// step 1. establish k8s client connection
client, contourClient, coordinationClient := newClient(ctx.Kubeconfig, ctx.InCluster)

// step 1b. get current namespace that Contour is running inside if not defined
if ctx.envoyServiceNamespace == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

setting ctx.envoyServiceName and ctx.envoyServiceNamespace are handled differently, is there a way to make them more similar?

Copy link
Member Author

Choose a reason for hiding this comment

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

The name come from a namespace, I'm using the check for empty to determine if a user hasn't already set the flag. If they haven't then I look up what namespace Contour is running in.

ctx.envoyServiceNamespace = getCurrentNamespace()
}

// step 2. create informers
// note: 0 means resync timers are disabled
coreInformers := coreinformers.NewSharedInformerFactory(client, 0)
Expand Down Expand Up @@ -158,8 +165,9 @@ func doServe(log logrus.FieldLogger, ctx *serveContext) error {
},
HoldoffDelay: 100 * time.Millisecond,
HoldoffMaxDelay: 500 * time.Millisecond,
CRDStatus: &k8s.CRDStatus{
Client: contourClient,
CRDStatus: &k8s.Status{
CRDClient: contourClient,
K8sClient: client,
},
Builder: dag.Builder{
Source: dag.KubernetesCache{
Expand All @@ -168,6 +176,7 @@ func doServe(log logrus.FieldLogger, ctx *serveContext) error {
FieldLogger: log.WithField("context", "KubernetesCache"),
},
DisablePermitInsecure: ctx.DisablePermitInsecure,
EnvoyNamespaceName: ctx.envoyServiceNamespace,
},
FieldLogger: log.WithField("context", "contourEventHandler"),
}
Expand Down
7 changes: 7 additions & 0 deletions cmd/contour/servecontext.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ type serveContext struct {
// ingress class
ingressClass string

// envoyServiceName
envoyServiceName string

// envoyServiceNamespace
envoyServiceNamespace string

// envoy's stats listener parameters
statsAddr string
statsPort int
Expand Down Expand Up @@ -164,6 +170,7 @@ func newServeContext() *serveContext {
Name: "leader-elect",
},
UseExtensionsV1beta1Ingress: false,
envoyServiceName: "envoy",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can envoyServiceNamespace default to projectcontour?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does if it cannot be looked up. I like the idea of requiring fewer flags to set on Contour, so I opted to look up the namespace which Contour is running in. If we take that out, we force users to set these flags and I can remove the lookup.

}
}

Expand Down
6 changes: 3 additions & 3 deletions internal/certgen/certgen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func WriteSecretsYAML(outputDir, namespace string, certdata map[string][]byte) e

// WriteSecretsKube writes all the keypairs out to Kube Secrets in the
// passed Kube context.
func WriteSecretsKube(client *kubernetes.Clientset, namespace string, certdata map[string][]byte) error {
func WriteSecretsKube(client kubernetes.Interface, namespace string, certdata map[string][]byte) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like an unrelated change, I think this file and the one above can be pulled out into their own PR to make this simpler to review.

Copy link
Member Author

Choose a reason for hiding this comment

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

See the previous comment about testing with fakes.

err := writeCACertKube(client, namespace, certdata["cacert.pem"])
if err != nil {
return err
Expand All @@ -99,7 +99,7 @@ func writeCACertSecret(outputDir, namespace string, cert []byte) error {
return checkFile(filename, writeSecret(f, secret))
}

func writeCACertKube(client *kubernetes.Clientset, namespace string, cert []byte) error {
func writeCACertKube(client kubernetes.Interface, namespace string, cert []byte) error {
secret := newCertOnlySecret("cacert", namespace, "cacert.pem", cert)
_, err := client.CoreV1().Secrets(namespace).Create(secret)
if err != nil {
Expand All @@ -123,7 +123,7 @@ func writeKeyPairSecret(outputDir, service, namespace string, cert, key []byte)
return checkFile(filepath, err)
}

func writeKeyPairKube(client *kubernetes.Clientset, service, namespace string, cert, key []byte) error {
func writeKeyPairKube(client kubernetes.Interface, service, namespace string, cert, key []byte) error {
secretname := service + "cert"
secret := newTLSSecret(secretname, namespace, key, cert)
_, err := client.CoreV1().Secrets(namespace).Create(secret)
Expand Down
55 changes: 33 additions & 22 deletions internal/contour/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type EventHandler struct {

HoldoffDelay, HoldoffMaxDelay time.Duration

CRDStatus *k8s.CRDStatus
CRDStatus *k8s.Status

*metrics.Metrics

Expand Down Expand Up @@ -239,32 +239,43 @@ func (e *EventHandler) updateDAG() {

// setStatus updates the status of objects.
func (e *EventHandler) setStatus(statuses map[dag.Meta]dag.Status) {
for _, st := range statuses {
switch obj := st.Object.(type) {
case *ingressroutev1.IngressRoute:
err := e.CRDStatus.SetStatus(st.Status, st.Description, obj)
if err != nil {
e.WithError(err).
WithField("status", st.Status).
WithField("desc", st.Description).
WithField("name", obj.Name).
WithField("namespace", obj.Namespace).
Error("failed to set status")
for _, status := range statuses {
switch st := status.(type) {
case dag.ObjectStatus:
switch obj := st.Object.(type) {
case *ingressroutev1.IngressRoute:
err := e.CRDStatus.SetStatus(st.Status, st.Description, obj)
if err != nil {
e.WithError(err).
WithField("status", st.Status).
WithField("desc", st.Description).
WithField("name", obj.Name).
WithField("namespace", obj.Namespace).
Error("failed to set status")
}
case *projcontour.HTTPProxy:
err := e.CRDStatus.SetStatus(st.Status, st.Description, obj)
if err != nil {
e.WithError(err).
WithField("status", st.Status).
WithField("desc", st.Description).
WithField("name", obj.Name).
WithField("namespace", obj.Namespace).
Error("failed to set status")
}
default:
e.WithField("namespace", obj.GetObjectMeta().GetNamespace()).
WithField("name", obj.GetObjectMeta().GetName()).
Error("set status: unknown object type")
}
case *projcontour.HTTPProxy:
err := e.CRDStatus.SetStatus(st.Status, st.Description, obj)
case dag.IngressStatus:
err := e.CRDStatus.SetIngressStatus(st.LoadBalancerIngress, &st.Object)
if err != nil {
e.WithError(err).
WithField("status", st.Status).
WithField("desc", st.Description).
WithField("name", obj.Name).
WithField("namespace", obj.Namespace).
WithField("name", st.Object.GetName()).
WithField("namespace", st.Object.Namespace).
Error("failed to set status")
}
default:
e.WithField("namespace", obj.GetObjectMeta().GetNamespace()).
WithField("name", obj.GetObjectMeta().GetName()).
Error("set status: unknown object type")
}
}
}
27 changes: 15 additions & 12 deletions internal/contour/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,20 @@ func calculateRouteMetric(statuses map[dag.Meta]dag.Status) (metrics.RouteMetric
proxyMetricOrphaned := make(map[metrics.Meta]int)
proxyMetricRoots := make(map[metrics.Meta]int)

for _, v := range statuses {
switch o := v.Object.(type) {
case *ingressroutev1.IngressRoute:
calcMetrics(v, irMetricValid, irMetricInvalid, irMetricOrphaned, irMetricTotal)
if o.Spec.VirtualHost != nil {
irMetricRoots[metrics.Meta{Namespace: v.Object.GetObjectMeta().GetNamespace()}]++
}
case *projcontour.HTTPProxy:
calcMetrics(v, proxyMetricValid, proxyMetricInvalid, proxyMetricOrphaned, proxyMetricTotal)
if o.Spec.VirtualHost != nil {
proxyMetricRoots[metrics.Meta{Namespace: v.Object.GetObjectMeta().GetNamespace()}]++
for _, s := range statuses {
switch v := s.(type) {
case dag.ObjectStatus:
switch o := v.Object.(type) {
case *ingressroutev1.IngressRoute:
calcMetrics(v, irMetricValid, irMetricInvalid, irMetricOrphaned, irMetricTotal)
if o.Spec.VirtualHost != nil {
irMetricRoots[metrics.Meta{Namespace: v.Object.GetObjectMeta().GetNamespace()}]++
}
case *projcontour.HTTPProxy:
calcMetrics(v, proxyMetricValid, proxyMetricInvalid, proxyMetricOrphaned, proxyMetricTotal)
if o.Spec.VirtualHost != nil {
proxyMetricRoots[metrics.Meta{Namespace: v.Object.GetObjectMeta().GetNamespace()}]++
}
}
}
}
Expand All @@ -67,7 +70,7 @@ func calculateRouteMetric(statuses map[dag.Meta]dag.Status) (metrics.RouteMetric
}
}

func calcMetrics(v dag.Status, metricValid map[metrics.Meta]int, metricInvalid map[metrics.Meta]int, metricOrphaned map[metrics.Meta]int, metricTotal map[metrics.Meta]int) {
func calcMetrics(v dag.ObjectStatus, metricValid map[metrics.Meta]int, metricInvalid map[metrics.Meta]int, metricOrphaned map[metrics.Meta]int, metricTotal map[metrics.Meta]int) {
switch v.Status {
case dag.StatusValid:
metricValid[metrics.Meta{VHost: v.Vhost, Namespace: v.Object.GetObjectMeta().GetNamespace()}]++
Expand Down
24 changes: 24 additions & 0 deletions internal/dag/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ type Builder struct {
// from which to build a DAG.
Source KubernetesCache

// EnvoyNamespaceName is the Kubernetes name+namespace where Envoy's service is deployed
EnvoyNamespaceName string

// DisablePermitInsecure disables the use of the
// permitInsecure field in IngressRoute.
DisablePermitInsecure bool
Expand Down Expand Up @@ -324,6 +327,10 @@ func (b *Builder) delegationPermitted(secret Meta, to string) bool {
}

func (b *Builder) computeIngresses() {

// holds the list of external lbs associated with the Envoy service
externalLB := GetExternalLB(b.EnvoyNamespaceName, b.Source.services)

// deconstruct each ingress into routes and virtualhost entries
for _, ing := range b.Source.ingresses {

Expand All @@ -332,7 +339,24 @@ func (b *Builder) computeIngresses() {
for _, rule := range rules {
b.computeIngressRule(ing, rule)
}

// Write status info to Ingress resource
sw, commit := b.WithIngressObject(*ing)
sw.SetLBStatus(externalLB)
commit()
}
}

func GetExternalLB(namespaceName string, services map[Meta]*v1.Service) []v1.LoadBalancerIngress {
var externalLB []v1.LoadBalancerIngress
// Lookup external lb information
for _, svc := range services {
// If this service has external status set and matches the Envoy service instance
if len(svc.Status.LoadBalancer.Ingress) > 0 && fmt.Sprintf("%s/%s", svc.GetNamespace(), svc.GetName()) == namespaceName {
externalLB = svc.Status.LoadBalancer.Ingress
}
}
return externalLB
}

func (b *Builder) computeIngressRule(ing *v1beta1.Ingress, rule v1beta1.IngressRule) {
Expand Down
Loading