-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Gke gcsfuse #7884
Gke gcsfuse #7884
Changes from 6 commits
31b9e30
eae3981
cafa96b
19a928a
f2c1020
f0c6a28
9004199
1e3f33c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3920,6 +3920,9 @@ resource "google_container_cluster" "primary" { | |
kalm_config { | ||
enabled = false | ||
} | ||
gcs_fuse_csi_driver_config { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can we fix the spacing on this (and the line above it) please, to be lined up with kalm_config |
||
enabled = false | ||
} | ||
<% end -%> | ||
} | ||
} | ||
|
@@ -3981,6 +3984,9 @@ resource "google_container_cluster" "primary" { | |
kalm_config { | ||
enabled = true | ||
} | ||
gcs_fuse_csi_driver_config { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: could we fix the spacing here too please? for both this and |
||
enabled = true | ||
} | ||
<% end -%> | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to only be available in beta, can we move all of this into
<% unless version == 'ga' -%>
blocks? You can follow howistio_config
andkalm_config
are handled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we move only
gcs_fuse_csi_driver_config
to 'ga' block or should we move everything inaddonsConfigKeys
?I understand that
istio_config
andkalm_config
are deprecated. What is the effect of movinggcs_fuse_csi_driver_config
to 'ga' block ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just
gcs_fuse_csi_driver_config
should move into the block. Typically in cases like these, I just move them into the existing blocks that containistio_config
andkalm_config
, thus they are all grouped together and its easier to read IMO. It shouldn't matter that the others are deprecated, they're still in the code and will be handled in the same manner wrt the provider. Does that make sense?Essentially the effect is that the new config block won't make it to the
google
provider, but only thegoogle-beta
provider. You can look at this comment above and it will show what the code will look like once they're generated downstream.For example, you'll see that
istio_config
is in thegoogle-beta
provider, but it doesn't exist in thegoogle
provider. (Except for docs, we standardize on beta docs)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the information.
I've updated it in the latest iteration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! and sorry for the mis-understanding, but you'll want to move all the code below that references it into blocks as well. if you do a search in the code for
istio_config
you'll see what i mean, i think.