feat: UseCase SetSkrKymaStateDeleting

[c-pius]

Description

Changes proposed in this pull request:

• implements UseCase SetSkrKymaStateDeleting

Related issue(s)

[lindnerby]

Not sure if this will work? the KCP Kyma will have a different namespace than the SKR Kyma obviously, but we are interested in the SKR Kyma status here, right?

[c-pius]

It works, but agree that it may be confusing. The thing is that the passed kcpKyma.GetNamespacedName() is required to get the related skr client. Once we have the client, the Get actually works on the default names.

Agree that this is not ideal, but also don't have a good idea right now how this could be made more obvious.

func (r *Repository) Get(ctx context.Context, kymaName types.NamespacedName) (*v1beta2.KymaStatus, error) {
	skrClient, err := r.getSkrClient(kymaName)
	if err != nil {
		return nil, err
	}

	kyma := &v1beta2.Kyma{}
	if err := skrClient.Get(ctx,
		types.NamespacedName{
			Name:      shared.DefaultRemoteKymaName,
			Namespace: shared.DefaultRemoteNamespace,
		},
		kyma,
	); err != nil {
		return nil, err
	}

	return &kyma.Status, nil
}

[c-pius]

Maybe it would be enough to state explicitly that the name we are expecting is kcpKymaName. Then it is obvious what to pass and the caller would rely on the repo to know how it translates this to the SKR kyma name?

[lindnerby]

💡 Now I get it. Question is now, why we include the namespace in the cache key, when it's always the same.

About the naming: Another idea could be to change from Get() into ResolveSkrStatusByKymaName(). Maybe too much, but it would include the client lookup part.
Just something to think about for now, while we continue to work on the feature branch.

[c-pius]

Create a TODO

[lindnerby]

In the context of the Repository this is actually the secret name,
the repo does not know that the secret name is the same as the kyma name. The consumer is deciding that and passing it. So secretName or name would be a better fit imo.

[c-pius]

I get the point. But let's discuss that shortly if you don't mind. I think it would also be valid to say that the repo knows how to map from a kymaName which is kind of the uniform identifier of the context we are reconciling in, to the dedicated resource we are touching. Same as the repo fir instance knows what namespace to look up in.

[lindnerby]

But I would argue that in this case we are literally looking up a Secret by its name. The Repo does not know the context and is pretty "dumb" here, although the Secret name is also the Kyma name in reality, this could also be an accident or change even, in this context it's not of any value rather confusing even maybe. But just my opinion 😄
Would you be fine with name?

[lindnerby]

Good point! We should for sure enforce this as a Singleton and make it apparent that it actually is the same reference used throughout kyma reconciliation. ☝️

[c-pius]

Yes. But I am also still contemplating with refactoring the client cache overall. Let's also quickly discuss where we put in our effort first 👍🏻