Skip to content

Conversation

@tombrk
Copy link
Member

@tombrk tombrk commented May 6, 2020

In 91bbd26, kausal was fixed to
ksonnet-lib/ksonnet.beta.4, which is problematic because:

  • it prevents use of other API compatible libraries, notably
    jsonnet-libs/k8s and kube-jsonnet/k
  • it forces users to use outdated, unmaintained, archived software
  • it bypasses a Tanka measure for this exact case, namely the "alias
    import" k.libsonnet, which has been explicitely set up for importing
    the Kubernetes library in the correct version for the users cluster.

Given that, I think we should revert most of these changes, keeping the
actual fix of that PR (a function signature backport).

I don't think this will have any impact on our internal config repo, as
it already uses ksonnet.beta.4 through the alias import, so
this should be a no-op.

In 91bbd26, kausal was fixed to
`ksonnet-lib/ksonnet.beta.4`, which is problematic because:

- it prevents use of other API compatible libraries, notably
`jsonnet-libs/k8s` and `kube-jsonnet/k`
- it forces users to use outdated, unmaintained, archived software
- it bypasses a Tanka measure for this exact case, namely the "alias
import" `k.libsonnet`, which has been explicitely set up for importing
the Kubernetes library in the correct version for the users cluster.

Given that, I think we should revert most of these changes, keeping the
actual fix of that PR (a function signature backport).

I don't think this will have any impact on our internal config repo, as
it already uses `ksonnet.beta.4` through the alias import, so
this *should* be a no-op.
@tombrk tombrk requested review from Duologic and tomwilkie May 6, 2020 22:45
@Duologic
Copy link
Contributor

Duologic commented May 7, 2020

I think kausal.libsonnet was written based on the archived ksonnet libs, that it works with other clones/forks seems to me a happy coincidence. I do agree it would be beneficial to support other clones/forks.

This proposed solution still dictates the existence and to some extend the location of a k.libsonnet, this is opinionated by Tanka. I think we can do better with Jsonnet, here are 2 options I can think of that will allow kausal.libsonnet to behave more like a composable library.

option 1

Initialize kausal.libsonnet, make the dependency explicit.

{
  init(k)::
    k {
      .... <kausal lib>
    }
}

Usage:

local k = import 'ksonnet.beta.4/k.libsonnet';
local kausal = import 'ksonnet-util/kausal.libsonnet';
kausal.init(k) + { ... }

option 2

Remove k from kausal.libsonnet, makes the dependency implicit, allow for direct import without initializing.

-local k = import 'ksonnet.beta.4/k.libsonnet';
-
-k {
+{

Usage:

local k = import 'ksonnet.beta.4/k.libsonnet';
local kausal = import 'ksonnet-util/kausal.libsonnet';
kausal + k + { ... }

My preference goes to option 1, importing the lib this way won't break anything in the tree by accident, a user has to explicitly initialize it to take advantage of the features.

@tombrk
Copy link
Member Author

tombrk commented May 7, 2020

(internal discussion result)

Both of above options aim to make it more proper, but are not backwards compatible. With more efforts they certainly could be made, but that might be not worth it, keeping in mind kausal.libsonnet is effectively subject to be phased out, as it's functionality will be integrated into the jsonnet-libs/k8s library.

The patch of this PR takes the short path of reverting to the previous behavior that served us well for months. While not most proper, it certainly gets the job done, with a minimal set of changes and things to watch out for.

@tombrk tombrk merged commit fddff10 into master May 7, 2020
@tombrk tombrk deleted the generalize-kausal branch May 7, 2020 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants