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.

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

Bastion host is immutable on 0.5.0 while it wasnt on 0.4.0 #1054

Closed
nikParasyr opened this issue Nov 19, 2021 · 12 comments · Fixed by #1070
Closed

Bastion host is immutable on 0.5.0 while it wasnt on 0.4.0 #1054

nikParasyr opened this issue Nov 19, 2021 · 12 comments · Fixed by #1070
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@nikParasyr
Copy link
Contributor

nikParasyr commented Nov 19, 2021

/kind bug

On a OpenStackCluster I ussualy config the bastion specs and set enable to false. I toggle the enable flag during troubleshooting or checking various components on the nodes. And i disable it again. This is rather useful to not waste capacity when its not required and also not have a backdoor to the nodes when its not required either.

What steps did you take and what happened:

  • upgrade to CAPO 0.5.0 and rest of the components to 1.0.1
  • toggle the bastion enable flag to true
  • error as it is immutable

What did you expect to happen:

  • create the bastion host as this was possible on 0.4.0

Environment:

  • Cluster API Provider OpenStack version (Or git rev-parse HEAD if manually built): 0.5.0
  • Cluster-API version: 1.0.1
  • OpenStack version: Stein
  • Kubernetes version (use kubectl version): many, shouldnt affect it
  • OS (e.g. from /etc/os-release): ubuntu20.04, shouldnt affect it either
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Nov 19, 2021
@hidekazuna
Copy link
Contributor

@nikParasyr Thank you for raising an issue. I agree the flag is useful for debugging purpose.

@tobiasgiese
Copy link
Member

tobiasgiese commented Dec 4, 2021

I'll take a look at it
/assign

Two questions @nikParasyr and @hidekazuna.
Should we allow the change to OpenStackCluser.Spec.Bastion at all, or just to the enabled flag?

Another question is, if we should allow changes to OpenStackCluserTemplate.Spec.Bastion. At least CAPA isn't allowing any changes to the AWSClusterTemplate.Spec.
https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/3e61428d885f2eaeab1a7dffc4d17270a20c6675/api/v1beta1/awsclustertemplate_webhook.go#L56-L64

@tobiasgiese
Copy link
Member

tobiasgiese commented Dec 4, 2021

I just created a PR.
We should discuss whether we want to allow changes to OpenStackCluserTemplate.Spec.Bastion or not.

@nikParasyr
Copy link
Contributor Author

Another question is, if we should allow changes to OpenStackCluserTemplate.Spec.Bastion.
We should discuss whether we want to allow changes to OpenStackCluserTemplate.Spec.Bastion or not.

I'd say theenabled flag is the most important. That being said, i can also see a user-case for long-running clusters where there might be a need to change flavor/image/image_uuid(if boot from volume) as well after some years of the cluster running.

At least CAPA isn't allowing any changes to the AWSClusterTemplate.Spec.

I dont think i can properly evaluate this and it's consequences tbh as i just recently started using cluster-api. From a user perspective I can see that there are some fields that it would be nice if they could be updated, e.g Tags as mentioned here

@tobiasgiese
Copy link
Member

tobiasgiese commented Dec 5, 2021

I updated the PR that changes to the complete OpenStackCluster.Spec.Bastion is possible.

Also I removed the allow changes to the OpenStackClusterTemplate.Spec at all. No other infrastructure provider (i.e., vsphere, aws, docker) is allowing these changes. Thus, I would suggest to keep it aligned. Further, the InfrastructureClusterTemplate (here OpenStackClusterTemplate) is part of ClusterClass (here is the proposal of ClusterClass).

Maybe there was a misunderstanding between OpenStackTemplate (the wording of your issue description) and OpenStackClusterTemplate?

@tobiasgiese
Copy link
Member

After we have a consensus regarding PR #1070 I'll create two further cherry-pick PRs for the release branches release-0.4 and release-0.5

@nikParasyr
Copy link
Contributor Author

Maybe there was a misunderstanding between OpenStackTemplate (the wording of your issue description) and OpenStackClusterTemplate?

You are right on this one. I meant OpenStackCluster and not template => thus it is not related to the ClusterClass stuff.
Ill update the description to avoid other misunderstandings

@hidekazuna
Copy link
Contributor

@tobiasgiese Thanks taking this issue. Yes, following other providers is good manner, since I implemented bastion watching CAPA and mainly CAPA and CAPZ are our upstream somehow.

@tobiasgiese
Copy link
Member

/reopen
Just to track the cherry-picks. I‘ll create them soon.

@k8s-ci-robot
Copy link
Contributor

@tobiasgiese: Reopened this issue.

In response to this:

/reopen
Just to track the cherry-picks. I‘ll create them soon.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tobiasgiese
Copy link
Member

Everything is merged
/close

@k8s-ci-robot
Copy link
Contributor

@tobiasgiese: Closing this issue.

In response to this:

Everything is merged
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
4 participants