Skip to content

Introduce inject check for known sidecars#1619

Merged
siggy merged 1 commit intomasterfrom
siggy/inject-known-sidecars
Sep 11, 2018
Merged

Introduce inject check for known sidecars#1619
siggy merged 1 commit intomasterfrom
siggy/inject-known-sidecars

Conversation

@siggy
Copy link
Member

@siggy siggy commented Sep 10, 2018

linkerd inject was not checking its input for known sidecars and
initContainers.

Modify linkerd inject to check for existing sidecars and
initContainers, specifically, Linkerd, Istio, and Contour.

Part of #1516

Signed-off-by: Andrew Seigner [email protected]

Depends on #1617

@siggy siggy added the area/cli label Sep 10, 2018
@siggy siggy self-assigned this Sep 10, 2018
@siggy siggy force-pushed the siggy/inject-known-sidecars branch from a7b114f to ea84e8a Compare September 11, 2018 17:21
@siggy siggy changed the base branch from siggy/inject-udp to master September 11, 2018 17:22
@siggy siggy force-pushed the siggy/inject-known-sidecars branch from ea84e8a to 75bda7b Compare September 11, 2018 17:28
Copy link

@rmars rmars left a comment

Choose a reason for hiding this comment

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

🌟 🏍 🚗

if len(sidecar) > 1 {
verb = "use"
}
output.Write([]byte(fmt.Sprintf("%s%s -- %s %s known sidecar\n", sidecarPrefix, warnStatus, strings.Join(sidecar, ", "), verb)))
Copy link

Choose a reason for hiding this comment

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

TIOLI you could use the text known sidecar detected in strings.Join(sidecar, ", ") (or known sidecar already present in ...) to avoid having the use/uses.

Copy link
Contributor

@klingerf klingerf left a comment

Choose a reason for hiding this comment

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

⭐️ Nice, looks good to me, but frankly I don't know enough about Contour to know if it's something we could work with or not.

`linkerd inject` was not checking its input for known sidecars and
initContainers.

Modify `linkerd inject` to check for existing sidecars and
initContainers, specifically, Linkerd, Istio, and Contour.

Part of #1516

Signed-off-by: Andrew Seigner <[email protected]>
@siggy siggy force-pushed the siggy/inject-known-sidecars branch from 75bda7b to 7858c95 Compare September 11, 2018 21:59
@siggy siggy merged commit 5d85680 into master Sep 11, 2018
@siggy siggy deleted the siggy/inject-known-sidecars branch September 11, 2018 22:09
zachalbert added a commit to zachalbert/linkerd2 that referenced this pull request Sep 13, 2018
* master:
  Update CHANGES.md for v18.9.1 release (linkerd#1631)
  Cleanly shutdown tap stream to data plane proxies (linkerd#1624)
  Change breadcrumb header to default font in styles.css (linkerd#1633)
  Improve top table to better cope with high RPS traffic (linkerd#1634)
  Add small success rate chart to table, misc web tweaks (linkerd#1628)
  Consolidate the source and destination columns in the Tap and Top tables (linkerd#1620)
  remove extraneous calc function in sidebar.css (linkerd#1632)
  Display more helpful websocket errors (linkerd#1626)
  Add breadcrumb navigation at the top of linkerd dashboard (linkerd#1613)
  Introduce inject check for known sidecars (linkerd#1619)
  Bump Prometheus to v2.4.0, Grafana to 5.2.4 (linkerd#1625)
  Improve performance of tap table by throttling updates (linkerd#1623)
  Add with-source flag to top (linkerd#1614)

Conflicts:
	web/app/css/styles.css
	web/app/js/components/ResourceDetail.jsx
	web/app/js/index.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants